fix memory handling in functional backend, add more error messages an…

…d comments for memory edgecases

kernel/functionalir.cc

@@ -461,43 +461,73 @@ class FunctionalIRConstruction {
 				memories[mem.cell] = &mem;
 		}
 	}
-	Node concatenate_read_results(Mem *, vector<Node> results)
+	Node concatenate_read_results(Mem *mem, vector<Node> results)
 	{
-        /* TODO: write code to check that this is ok to do */
-		if(results.size() == 0)
-			return factory.undriven(0);
+		// sanity check: all read ports concatenated should equal to the RD_DATA port
+		const SigSpec &rd_data = mem->cell->connections().at(ID(RD_DATA));
+		int current = 0;
+		for(size_t i = 0; i < mem->rd_ports.size(); i++) {
+			int width = mem->width << mem->rd_ports[i].wide_log2;
+			log_assert (results[i].width() == width);
+			log_assert (mem->rd_ports[i].data == rd_data.extract(current, width));
+			current += width;
+		}
+		log_assert (current == rd_data.size());
+		log_assert (!results.empty());
 		Node node = results[0];
 		for(size_t i = 1; i < results.size(); i++)
 			node = factory.concat(node, results[i]);
 		return node;
 	}
 	Node handle_memory(Mem *mem)
 	{
+		// To simplify memory handling, the functional backend makes the following assumptions:
+		// - Since async2sync or clk2fflogic must be run to use the functional backend,
+		//   we can assume that all ports are asynchronous.
+		// - Async rd/wr are always transparent and so we must do reads after writes,
+		//   but we can ignore transparency_mask.
+		// - We ignore collision_x_mask because x is a dont care value for us anyway.
+		// - Since wr port j can only have priority over wr port i if j > i, if we do writes in
+		//   ascending index order the result will obey the priorty relation.
 		vector<Node> read_results;
 		int addr_width = ceil_log2(mem->size);
 		int data_width = mem->width;
 		Node node = factory.state_memory(mem->cell->name, addr_width, data_width);
-		for (auto &rd : mem->rd_ports) {
-			log_assert(!rd.clk_enable);
-			Node addr = enqueue(driver_map(DriveSpec(rd.addr)));
-			read_results.push_back(factory.memory_read(node, addr));
-		}
-		for (auto &wr : mem->wr_ports) {
+		for (size_t i = 0; i < mem->wr_ports.size(); i++) {
+			const auto &wr = mem->wr_ports[i];
+			if (wr.clk_enable)
+				log_error("Write port %zd of memory %s.%s is clocked. This is not supported by the functional backend. "
+					"Call async2sync or clk2fflogic to avoid this error.\n", i, log_id(mem->module), log_id(mem->memid));
 			Node en = enqueue(driver_map(DriveSpec(wr.en)));
 			Node addr = enqueue(driver_map(DriveSpec(wr.addr)));
 			Node new_data = enqueue(driver_map(DriveSpec(wr.data)));
 			Node old_data = factory.memory_read(node, addr);
 			Node wr_data = simplifier.bitwise_mux(old_data, new_data, en);
 			node = factory.memory_write(node, addr, wr_data);
 		}
+		if (mem->rd_ports.empty())
+			log_error("Memory %s.%s has no read ports. This is not supported by the functional backend. "
+				"Call opt_clean to remove it.", log_id(mem->module), log_id(mem->memid));
+		for (size_t i = 0; i < mem->rd_ports.size(); i++) {
+			const auto &rd = mem->rd_ports[i];
+			if (rd.clk_enable)
+				log_error("Read port %zd of memory %s.%s is clocked. This is not supported by the functional backend. "
+					"Call memory_nordff to avoid this error.\n", i, log_id(mem->module), log_id(mem->memid));
+			Node addr = enqueue(driver_map(DriveSpec(rd.addr)));
+			read_results.push_back(factory.memory_read(node, addr));
+		}
 		factory.declare_state_memory(node, mem->cell->name, addr_width, data_width);
 		return concatenate_read_results(mem, read_results);
 	}
 	void process_cell(Cell *cell)
 	{
 		if (cell->is_mem_cell()) {
 			Mem *mem = memories.at(cell, nullptr);
-			log_assert(mem != nullptr);
+			if (mem == nullptr) {
+				log_assert(cell->has_memid());
+				log_error("The design contains an unpacked memory at %s. This is not supported by the functional backend. "
+					"Call memory_collect to avoid this error.\n", log_const(cell->parameters.at(ID(MEMID))));
+			}
 			Node node = handle_memory(mem);
 			factory.update_pending(cell_outputs.at({cell, ID(RD_DATA)}), node);
 		} else {

tests/functional/rtlil_cells.py

@@ -176,14 +176,52 @@ def write_rtlil_file(self, path, parameters):
     input wire clk,
     input wire [{1}:0] WA,
     input wire [{0}:0] WD,
+    input wire [{1}:0] RA,
     output reg [{0}:0] RD
 );
-    reg [{0}:0] mem[0:{1}];
+    reg [{0}:0] mem[0:{2}];
     always @(*)
         RD = mem[RA];
     always @(posedge clk)
         mem[WA] <= WD;
-endmodule""".format(parameters['DATA_WIDTH'] - 1, parameters['ADDR_WIDTH'] - 1))
+endmodule""".format(parameters['DATA_WIDTH'] - 1, parameters['ADDR_WIDTH'] - 1, 2**parameters['ADDR_WIDTH'] - 1))
+        yosys_synth(verilog_file, path)
+
+class MemDualCell(BaseCell):
+    def __init__(self, name, values):
+        super().__init__(name, ['DATA_WIDTH', 'ADDR_WIDTH'],
+                         {'WA1': 'ADDR_WIDTH', 'WA2': 'ADDR_WIDTH',
+                          'RA1': 'ADDR_WIDTH', 'RA2': 'ADDR_WIDTH',
+                          'WD1': 'DATA_WIDTH', 'WD2': 'DATA_WIDTH'},
+                         {'RD1': 'DATA_WIDTH', 'RD2': 'DATA_WIDTH'}, values)
+        self.sim_preprocessing = "memory_map" # issue #4496 in yosys -sim prevents this example from working without memory_map
+    def write_rtlil_file(self, path, parameters):
+        from test_functional import yosys_synth
+        verilog_file = path.parent / 'verilog.v'
+        with open(verilog_file, 'w') as f:
+            f.write("""
+module gold(
+    input wire clk,
+    input wire [{1}:0] WA1,
+    input wire [{0}:0] WD1,
+    input wire [{1}:0] WA2,
+    input wire [{0}:0] WD2,
+    input wire [{1}:0] RA1,
+    input wire [{1}:0] RA2,
+    output reg [{0}:0] RD1,
+    output reg [{0}:0] RD2
+);
+                    (*keep*)
+    reg [{0}:0] mem[0:{2}];
+    always @(*)
+        RD1 = mem[RA1];
+    always @(*)
+        RD2 = mem[RA2];
+    always @(posedge clk) begin
+        mem[WA1] <= WD1;
+        mem[WA2] <= WD2;
+    end
+endmodule""".format(parameters['DATA_WIDTH'] - 1, parameters['ADDR_WIDTH'] - 1, 2**parameters['ADDR_WIDTH'] - 1))
         yosys_synth(verilog_file, path)
 
 binary_widths = [
@@ -284,7 +322,8 @@ def write_rtlil_file(self, path, parameters):
     BWCell("bweqx", [10, 16, 40]),
     BWCell("bwmux", [10, 16, 40]),
     FFCell("ff", [10, 20, 40]),
-    MemCell("mem", [(32, 4)])
+    MemCell("mem", [(16, 4)]),
+    MemDualCell("mem-dual", [(16, 4)]),
 #    ("assert", ["A", "EN"]),
 #    ("assume", ["A", "EN"]),
 #    ("live", ["A", "EN"]),

tests/functional/test_functional.py

@@ -24,7 +24,7 @@ def compile_cpp(in_path, out_path, args):
     run(['g++', '-g', '-std=c++17'] + args + [str(in_path), '-o', str(out_path)])
 
 def yosys_synth(verilog_file, rtlil_file):
-    yosys(f"read_verilog {quote(verilog_file)} ; prep ; clk2fflogic ; write_rtlil {quote(rtlil_file)}")
+    yosys(f"read_verilog {quote(verilog_file)} ; prep ; write_rtlil {quote(rtlil_file)}")
 
 # simulate an rtlil file with yosys, comparing with a given vcd file, and writing out the yosys simulation results into a second vcd file
 def yosys_sim(rtlil_file, vcd_reference_file, vcd_out_file, preprocessing = ""):
@@ -49,7 +49,7 @@ def test_cxx(cell, parameters, tmp_path, num_steps, rnd):
     vcd_yosys_sim_file = tmp_path / 'yosys.vcd'
 
     cell.write_rtlil_file(rtlil_file, parameters)
-    yosys(f"read_rtlil {quote(rtlil_file)} ; write_functional_cxx {quote(cc_file)}")
+    yosys(f"read_rtlil {quote(rtlil_file)} ; clk2fflogic ; write_functional_cxx {quote(cc_file)}")
     compile_cpp(vcdharness_cc_file, vcdharness_exe_file, ['-I', tmp_path, '-I', str(base_path / 'backends/functional/cxx_runtime')])
     seed = str(rnd(cell.name + "-cxx").getrandbits(32))
     run([str(vcdharness_exe_file.resolve()), str(vcd_functional_file), str(num_steps), str(seed)])
@@ -64,7 +64,7 @@ def test_smt(cell, parameters, tmp_path, num_steps, rnd):
     vcd_yosys_sim_file = tmp_path / 'yosys.vcd'
 
     cell.write_rtlil_file(rtlil_file, parameters)
-    yosys(f"read_rtlil {quote(rtlil_file)} ; write_functional_smt2 {quote(smt_file)}")
+    yosys(f"read_rtlil {quote(rtlil_file)} ; clk2fflogic ; write_functional_smt2 {quote(smt_file)}")
     run(['z3', smt_file]) # check if output is valid smtlib before continuing
     smt_vcd.simulate_smt(smt_file, vcd_functional_file, num_steps, rnd(cell.name + "-smt"))
     yosys_sim(rtlil_file, vcd_functional_file, vcd_yosys_sim_file, getattr(cell, 'sim_preprocessing', ''))