Fix read-compute-write tests

This gives us a single runt test that goes from
a calyx vector add to its expected output after
being simulated with cocotb. Can replace a lot of what
was removed in #2515

Author: nathanielnrn

runt.toml

@@ -671,3 +671,18 @@ make --silent -B TEST_PATH={} COCOTB_LOG_LEVEL=CRITICAL |\
   rm -f results.xml
 
 """
+
+
+# Assumes same basename for .futil and .yxi
+# TODO(nnavarro): Change paths to `yxi/tests/axi/*/*-vec-add.futil` when calyx-AXI wrapper is fixed
+[[tests]]
+name = "Cocotb axi wrapper correctness test"
+paths = ["yxi/tests/axi/read-compute-write/*-vec-add.futil"]
+cmd = """
+ dynamic=$(head -n 2 {} | tail -n 1 | cut -c 4-) && \
+ dirname=$(dirname {}) && \
+ filename=$(basename {} .futil) && \
+ fud2 {} --to calyx --through axi-wrapped --set ${dynamic} -o axi-wrapped.futil 2> /dev/null && \
+ fud2 axi-wrapped.futil --from calyx --to verilog-noverify -o axi-wrapped.v 2> /dev/null && \
+ fud2 axi-wrapped.v --from verilog-noverify --to cocotb-axi --set sim.data=${dirname}/../vectorized-add.data 2> /dev/null
+ """

yxi/axi-calyx/axi_controller_generator.py

@@ -185,9 +185,9 @@ def add_bresp_channel(prog):
 
     with bresp_channel.continuous:
         bresp_channel.this()["BVALID"] = bvalid.out
-        bresp_channel.this()["BRESP"] = (
-            0b00  # Assume OKAY. Could make this dynamic in the future by passing in a ref cell.
-        )
+        bresp_channel.this()[
+            "BRESP"
+        ] = 0b00  # Assume OKAY. Could make this dynamic in the future by passing in a ref cell.
 
     with bresp_channel.group("block_transfer") as block_transfer:
         BREADY = bresp_channel.this()["BREADY"]
@@ -593,16 +593,16 @@ def build():
 def check_mems_welformed(mems):
     """Checks if memories from yxi are well formed. Returns true if they are, false otherwise."""
     for mem in mems:
-        assert mem[width_key] % 8 == 0, (
-            "Width must be a multiple of 8 to alow byte addressing to host"
-        )
-        assert log2(mem[width_key]).is_integer(), (
-            "Width must be a power of 2 to be correctly described by xSIZE"
-        )
+        assert (
+            mem[width_key] % 8 == 0
+        ), "Width must be a multiple of 8 to alow byte addressing to host"
+        assert log2(
+            mem[width_key]
+        ).is_integer(), "Width must be a power of 2 to be correctly described by xSIZE"
         assert mem[size_key] > 0, "Memory size must be greater than 0"
-        assert mem[type_key] == "Dynamic", (
-            "Only dynamic memories are currently supported for dynamic axi"
-        )
+        assert (
+            mem[type_key] == "Dynamic"
+        ), "Only dynamic memories are currently supported for dynamic axi"
 
 
 if __name__ == "__main__":

yxi/axi-calyx/axi_generator.py

@@ -446,7 +446,7 @@ def add_bresp_channel(prog, mem):
 
 # NOTE: Unlike the channel functions, this can expect multiple mems
 def add_main_comp(prog, mems):
-    wrapper_comp = prog.component("wrapper")
+    wrapper_comp = prog.component("Toplevel")
     wrapper_comp.attribute("toplevel", 1)
     # Get handles to be used later
     read_channel = prog.get_component("m_read_channel")
@@ -468,52 +468,54 @@ def add_main_comp(prog, mems):
     # Naming the clock signal `ap_clk` ensures Xilinx tool compatability
     wrapper_comp.input("ap_clk", 1, ["clk"])
 
-    # TODO: To get these ports to interface with XRT and our current xml_generator.py we need to add a
-    # `prefixed_mem_name = f"m_axi_{mem[name_key]}"`
-    # and replace most (but not all) usages of {mem_name} with `prefixed_mem_name`.
-
     for mem in mems:
+
         mem_name = mem[name_key]
+        # These input/output names in the toplevel (i.e. 'm_axi_A0_ARREADY') need to match
+        # the ports(not!! args) kernel.xml file generated by `xml_generator.py`.
+        # We add the prefix `m_axi_` to maintain compatibility with the old verilog wrapper.
+        # Once we deprecate the old wrapper we can probably remove this prefix here and modify `xml_generator.py`
+        prefixed_mem_name = f"m_axi_{mem[name_key]}"
         # Inputs/Outputs
         wrapper_inputs = [
-            (f"{mem_name}_ARESETn", 1),
-            (f"{mem_name}_ARREADY", 1),
-            (f"{mem_name}_RVALID", 1),
-            (f"{mem_name}_RLAST", 1),
-            (f"{mem_name}_RDATA", mem[width_key]),
-            (f"{mem_name}_RRESP", 2),
-            (f"{mem_name}_AWREADY", 1),
-            (f"{mem_name}_WRESP", 2),
-            (f"{mem_name}_WREADY", 1),
-            (f"{mem_name}_BVALID", 1),
+            (f"{prefixed_mem_name}_ARESETn", 1),
+            (f"{prefixed_mem_name}_ARREADY", 1),
+            (f"{prefixed_mem_name}_RVALID", 1),
+            (f"{prefixed_mem_name}_RLAST", 1),
+            (f"{prefixed_mem_name}_RDATA", mem[width_key]),
+            (f"{prefixed_mem_name}_RRESP", 2),
+            (f"{prefixed_mem_name}_AWREADY", 1),
+            (f"{prefixed_mem_name}_WRESP", 2),
+            (f"{prefixed_mem_name}_WREADY", 1),
+            (f"{prefixed_mem_name}_BVALID", 1),
             # Only used for waveform tracing, not sent anywhere
-            (f"{mem_name}_BRESP", 2),
+            (f"{prefixed_mem_name}_BRESP", 2),
             # Only needed for coctb compatability, tied low
-            (f"{mem_name}_RID", 1),
+            (f"{prefixed_mem_name}_RID", 1),
         ]
 
         wrapper_outputs = [
-            (f"{mem_name}_ARVALID", 1),
-            (f"{mem_name}_ARADDR", 64),
-            (f"{mem_name}_ARSIZE", 3),
-            (f"{mem_name}_ARLEN", 8),
-            (f"{mem_name}_ARBURST", 2),
-            (f"{mem_name}_RREADY", 1),
-            (f"{mem_name}_AWVALID", 1),
-            (f"{mem_name}_AWADDR", 64),
-            (f"{mem_name}_AWSIZE", 3),
-            (f"{mem_name}_AWLEN", 8),
-            (f"{mem_name}_AWBURST", 2),
-            (f"{mem_name}_AWPROT", 3),
-            (f"{mem_name}_WVALID", 1),
-            (f"{mem_name}_WLAST", 1),
-            (f"{mem_name}_WDATA", mem[width_key]),
-            (f"{mem_name}_BREADY", 1),
+            (f"{prefixed_mem_name}_ARVALID", 1),
+            (f"{prefixed_mem_name}_ARADDR", 64),
+            (f"{prefixed_mem_name}_ARSIZE", 3),
+            (f"{prefixed_mem_name}_ARLEN", 8),
+            (f"{prefixed_mem_name}_ARBURST", 2),
+            (f"{prefixed_mem_name}_RREADY", 1),
+            (f"{prefixed_mem_name}_AWVALID", 1),
+            (f"{prefixed_mem_name}_AWADDR", 64),
+            (f"{prefixed_mem_name}_AWSIZE", 3),
+            (f"{prefixed_mem_name}_AWLEN", 8),
+            (f"{prefixed_mem_name}_AWBURST", 2),
+            (f"{prefixed_mem_name}_AWPROT", 3),
+            (f"{prefixed_mem_name}_WVALID", 1),
+            (f"{prefixed_mem_name}_WLAST", 1),
+            (f"{prefixed_mem_name}_WDATA", mem[width_key]),
+            (f"{prefixed_mem_name}_BREADY", 1),
             # ID signals are needed for coco compatability, tied low
-            (f"{mem_name}_ARID", 1),
-            (f"{mem_name}_AWID", 1),
-            (f"{mem_name}_WID", 1),
-            (f"{mem_name}_BID", 1),
+            (f"{prefixed_mem_name}_ARID", 1),
+            (f"{prefixed_mem_name}_AWID", 1),
+            (f"{prefixed_mem_name}_WID", 1),
+            (f"{prefixed_mem_name}_BID", 1),
         ]
 
         add_comp_ports(wrapper_comp, wrapper_inputs, wrapper_outputs)
@@ -547,10 +549,10 @@ def add_main_comp(prog, mems):
 
         # Tie IDs low, needed for cocotb compatability. Not used anywhere
         with wrapper_comp.continuous:
-            wrapper_comp.this()[f"{mem_name}_ARID"] = 0
-            wrapper_comp.this()[f"{mem_name}_AWID"] = 0
-            wrapper_comp.this()[f"{mem_name}_WID"] = 0
-            wrapper_comp.this()[f"{mem_name}_BID"] = 0
+            wrapper_comp.this()[f"{prefixed_mem_name}_ARID"] = 0
+            wrapper_comp.this()[f"{prefixed_mem_name}_AWID"] = 0
+            wrapper_comp.this()[f"{prefixed_mem_name}_WID"] = 0
+            wrapper_comp.this()[f"{prefixed_mem_name}_BID"] = 0
 
         # No groups needed!
 
@@ -562,41 +564,41 @@ def add_main_comp(prog, mems):
             # main_comp.get_cell(f"ar_channel_{mem_name}"),
             wrapper_comp.get_cell(f"ar_channel_{mem_name}"),
             ref_curr_addr_axi=curr_addr_axi,
-            in_ARESETn=this_component[f"{mem_name}_ARESETn"],
-            in_ARREADY=this_component[f"{mem_name}_ARREADY"],
-            out_ARVALID=this_component[f"{mem_name}_ARVALID"],
-            out_ARADDR=this_component[f"{mem_name}_ARADDR"],
-            out_ARSIZE=this_component[f"{mem_name}_ARSIZE"],
-            out_ARLEN=this_component[f"{mem_name}_ARLEN"],
-            out_ARBURST=this_component[f"{mem_name}_ARBURST"],
+            in_ARESETn=this_component[f"{prefixed_mem_name}_ARESETn"],
+            in_ARREADY=this_component[f"{prefixed_mem_name}_ARREADY"],
+            out_ARVALID=this_component[f"{prefixed_mem_name}_ARVALID"],
+            out_ARADDR=this_component[f"{prefixed_mem_name}_ARADDR"],
+            out_ARSIZE=this_component[f"{prefixed_mem_name}_ARSIZE"],
+            out_ARLEN=this_component[f"{prefixed_mem_name}_ARLEN"],
+            out_ARBURST=this_component[f"{prefixed_mem_name}_ARBURST"],
         )
 
         read_channel_invoke = invoke(
             wrapper_comp.get_cell(f"read_channel_{mem_name}"),
             ref_mem_ref=internal_mem,
             ref_curr_addr_internal_mem=curr_addr_internal_mem,
             ref_curr_addr_axi=curr_addr_axi,
-            in_ARESETn=this_component[f"{mem_name}_ARESETn"],
-            in_RVALID=this_component[f"{mem_name}_RVALID"],
-            in_RLAST=this_component[f"{mem_name}_RLAST"],
-            in_RDATA=this_component[f"{mem_name}_RDATA"],
+            in_ARESETn=this_component[f"{prefixed_mem_name}_ARESETn"],
+            in_RVALID=this_component[f"{prefixed_mem_name}_RVALID"],
+            in_RLAST=this_component[f"{prefixed_mem_name}_RLAST"],
+            in_RDATA=this_component[f"{prefixed_mem_name}_RDATA"],
             # TODO: Do we need this? Don't think this goes anywhere
-            in_RRESP=this_component[f"{mem_name}_RRESP"],
-            out_RREADY=this_component[f"{mem_name}_RREADY"],
+            in_RRESP=this_component[f"{prefixed_mem_name}_RRESP"],
+            out_RREADY=this_component[f"{prefixed_mem_name}_RREADY"],
         )
 
         aw_channel_invoke = invoke(
             wrapper_comp.get_cell(f"aw_channel_{mem_name}"),
             ref_curr_addr_axi=curr_addr_axi,
             ref_max_transfers=max_transfers,
-            in_ARESETn=this_component[f"{mem_name}_ARESETn"],
-            in_AWREADY=this_component[f"{mem_name}_AWREADY"],
-            out_AWVALID=this_component[f"{mem_name}_AWVALID"],
-            out_AWADDR=this_component[f"{mem_name}_AWADDR"],
-            out_AWSIZE=this_component[f"{mem_name}_AWSIZE"],
-            out_AWLEN=this_component[f"{mem_name}_AWLEN"],
-            out_AWBURST=this_component[f"{mem_name}_AWBURST"],
-            out_AWPROT=this_component[f"{mem_name}_AWPROT"],
+            in_ARESETn=this_component[f"{prefixed_mem_name}_ARESETn"],
+            in_AWREADY=this_component[f"{prefixed_mem_name}_AWREADY"],
+            out_AWVALID=this_component[f"{prefixed_mem_name}_AWVALID"],
+            out_AWADDR=this_component[f"{prefixed_mem_name}_AWADDR"],
+            out_AWSIZE=this_component[f"{prefixed_mem_name}_AWSIZE"],
+            out_AWLEN=this_component[f"{prefixed_mem_name}_AWLEN"],
+            out_AWBURST=this_component[f"{prefixed_mem_name}_AWBURST"],
+            out_AWPROT=this_component[f"{prefixed_mem_name}_AWPROT"],
         )
 
         write_channel_invoke = invoke(
@@ -605,17 +607,17 @@ def add_main_comp(prog, mems):
             ref_curr_addr_internal_mem=curr_addr_internal_mem,
             ref_curr_addr_axi=curr_addr_axi,
             ref_max_transfers=max_transfers,
-            in_ARESETn=this_component[f"{mem_name}_ARESETn"],
-            in_WREADY=this_component[f"{mem_name}_WREADY"],
-            out_WVALID=this_component[f"{mem_name}_WVALID"],
-            out_WLAST=this_component[f"{mem_name}_WLAST"],
-            out_WDATA=this_component[f"{mem_name}_WDATA"],
+            in_ARESETn=this_component[f"{prefixed_mem_name}_ARESETn"],
+            in_WREADY=this_component[f"{prefixed_mem_name}_WREADY"],
+            out_WVALID=this_component[f"{prefixed_mem_name}_WVALID"],
+            out_WLAST=this_component[f"{prefixed_mem_name}_WLAST"],
+            out_WDATA=this_component[f"{prefixed_mem_name}_WDATA"],
         )
 
         bresp_channel_invoke = invoke(
             wrapper_comp.get_cell(f"bresp_channel_{mem_name}"),
-            in_BVALID=this_component[f"{mem_name}_BVALID"],
-            out_BREADY=this_component[f"{mem_name}_BREADY"],
+            in_BVALID=this_component[f"{prefixed_mem_name}_BVALID"],
+            out_BREADY=this_component[f"{prefixed_mem_name}_BREADY"],
         )
 
         curr_addr_axi_invoke = invoke(curr_addr_axi, in_in=0x1000)
@@ -690,12 +692,12 @@ def build():
 def check_mems_welformed(mems):
     """Checks if memories from yxi are well formed. Returns true if they are, false otherwise."""
     for mem in mems:
-        assert mem[width_key] % 8 == 0, (
-            "Width must be a multiple of 8 to alow byte addressing to host"
-        )
-        assert log2(mem[width_key]).is_integer(), (
-            "Width must be a power of 2 to be correctly described by xSIZE"
-        )
+        assert (
+            mem[width_key] % 8 == 0
+        ), "Width must be a multiple of 8 to alow byte addressing to host"
+        assert log2(
+            mem[width_key]
+        ).is_integer(), "Width must be a power of 2 to be correctly described by xSIZE"
         assert mem[size_key] > 0, "Memory size must be greater than 0"
 
 

yxi/axi-calyx/cocotb/Makefile

@@ -13,7 +13,7 @@ VERILOG_SOURCES += $(VERILOG_SOURCE)
 SIM_BUILD=sim_build/
 
 # TOPLEVEL is the name of the toplevel module in your Verilog or VHDL file
-TOPLEVEL = wrapper
+TOPLEVEL = Toplevel
 
 # MODULE is the basename of the Python test file
 MODULE = run_axi_test

yxi/axi-calyx/cocotb/run_axi_test.py

@@ -14,10 +14,10 @@ async def run(toplevel):
     from axi_test import run_kernel_test
 
     data_path = os.environ.get("DATA_PATH")
-    if not os.path.isabs(data_path):
+    if data_path and not os.path.isabs(data_path):
         data_path = os.getcwd() + "/" + data_path
-    assert data_path is not None and os.path.isfile(data_path), (
-        "DATA_PATH must be set and must be a valid file."
-    )
+    assert data_path is not None and os.path.isfile(
+        data_path
+    ), f"DATA_PATH must be set and must be a valid file. Got: {data_path}"
 
     await run_kernel_test(toplevel, data_path)

yxi/axi-calyx/dynamic_axi_generator.py

@@ -903,16 +903,16 @@ def build():
 def check_mems_wellformed(mems):
     """Checks if memories from yxi are well formed. Returns true if they are, false otherwise."""
     for mem in mems:
-        assert mem[width_key] % 8 == 0, (
-            "Width must be a multiple of 8 to alow byte addressing to host"
-        )
-        assert log2(mem[width_key]).is_integer(), (
-            "Width must be a power of 2 to be correctly described by xSIZE"
-        )
+        assert (
+            mem[width_key] % 8 == 0
+        ), "Width must be a multiple of 8 to alow byte addressing to host"
+        assert log2(
+            mem[width_key]
+        ).is_integer(), "Width must be a power of 2 to be correctly described by xSIZE"
         assert mem[size_key] > 0, "Memory size must be greater than 0"
-        assert mem[type_key] == "Dynamic", (
-            "Only dynamic memories are currently supported for dynamic axi"
-        )
+        assert (
+            mem[type_key] == "Dynamic"
+        ), "Only dynamic memories are currently supported for dynamic axi"
 
 
 if __name__ == "__main__":

yxi/tests/axi/read-compute-write/seq-vec-add.expect

@@ -0,0 +1,53 @@
+{
+  "A0": {
+    "data": [
+      1,
+      3,
+      7,
+      15,
+      31,
+      63,
+      127,
+      255
+    ],
+    "format": {
+      "numeric_type": "bitnum",
+      "is_signed": false,
+      "width": 32
+    }
+  },
+  "B0": {
+    "data": [
+      1,
+      1,
+      1,
+      1,
+      1,
+      1,
+      1,
+      1
+    ],
+    "format": {
+      "numeric_type": "bitnum",
+      "is_signed": false,
+      "width": 32
+    }
+  },
+  "Sum0": {
+    "data": [
+      2,
+      4,
+      8,
+      16,
+      32,
+      64,
+      128,
+      256
+    ],
+    "format": {
+      "numeric_type": "bitnum",
+      "is_signed": false,
+      "width": 32
+    }
+  }
+}