WIP: Create a backwards compatible flow for `xo`/`xclbin` generation with Calyx-AXI-wrapper #2267

nathanielnrn · 2024-08-20T05:20:21Z

At the highest level this PR should allow for correct xclbins to be produced from Calyx programs.

Broadly speaking this PR does 2 things (sorry for combining them, but some small fixes are sprinkled throughout making things hard to separate into their own changes):

This PR introduces an axi_controller_generator.py which is meant to create a subordinate adhering to Xilinx's control spec. There might be some issues with this as executing on hardwares seems to hang at the moment, and the only difference between the generated verilog is the addition of the controller subordinate AFAIK.
Adds some fud2 functions to go from verilog to a .xo. This is different from the previous .futil to .xo (which should eventually be deprecated), and is the "correct" way to create a .xo with the new Calyx-AXI-wrappers.

I'll do my best to comment the PR heavily to explain which changes affect what.

Furthermore, not sure if the controller itself needs a deep dive w.r.t code review. There are probably problems with it, that require waveform debugging to uncover.

EDIT: More work than I thought regarding backwards compatibility. Commented where changes are needed

In general there are a few outstanding TODOs even after this PR is merged. I'll make issues about these:

Update the static wrapper generator to correctly add a Xilinx control subordinate to the wrapper.
Change axi-generator.py to axi_generator.py
Test that xclbins get produced correctly with our static wrapper (currently this has mainly been tested on dynamic versions.
Figure out why the current dynamic wrapper seems to hang when running on actual hardware (likely an issue with the control subordinate)

…control of a component

TODO: Hook up the slices in the highest level module for ap_done, ap_start. Also thinnk about go done signals/how these connect to rest of wrapper

* Prepare fifo for case idiom * Attempt with . Failing due to empty control * Delete calyx-py/test/correctness/queues/fifo.futil

…n builder

…statements

TODO: Hook up ap_start from controller in wrapper to the main_compute module

This works around #2198 causing combinational loops

…lob does not find any .v files

…evel'. This generator will not work with the old verilog-wrapper

This is no longer an argument to our YXI-wrapped kernels; it only exists in the old Verilog-generator stuff. We maybe want to revert/make this optional at some point to make `xclrun` compatible with the old version too?

This allows us to not change the `kernel.xml` we use in the old wrapper vs the new wrapper and still get correct results

nathanielnrn · 2024-08-20T05:22:26Z

calyx-py/calyx/builder.py

-    def output_with_attributes(
-        self,
-        name: str,
-        size: int,
-        attribute_literals: List[Union[str, Tuple[str, int]]],
-    ) -> ExprBuilder:
-        """Declare an output port on the component with attributes.
-
-        Returns an expression builder for the port.
-        """
-        return self._port_with_attributes(name, size, False, attribute_literals)
-


This is some unrelated cleanup, the output method already exists and does the same thing

nathanielnrn · 2024-08-20T05:55:26Z

fud2/rsrc/gen_xo.tcl

@@ -43,4 +43,4 @@ ipx::save_core [ipx::current_core]
 close_project -delete

 # Package the project as an .xo file.
-package_xo -xo_path ${xoname} -kernel_name Toplevel -ip_directory ${path_to_packaged} -kernel_xml ./kernel.xml
+package_xo -xo_path ${xoname} -kernel_name wrapper -ip_directory ${path_to_packaged} -kernel_xml ./kernel.xml


TODO: Another breaking change from the verilog wrapper. probably just change the name of wrapper to Toplevel

nathanielnrn · 2024-08-20T05:56:39Z

fud2/scripts/axi.rhai

This whole file is largely just clean up, getting rid of dead code/simplifying some sed commands

nathanielnrn · 2024-08-20T05:58:27Z

fud2/scripts/xilinx.rhai

These are the fud2 changes

nathanielnrn · 2024-08-20T05:59:32Z

yxi/axi-calyx/cocotb/axi_test.py

    timeout = 5000
+    #Base addresses for memories
+    await tb.control_manager.write(0x0010, encode([0x0],4))
+    await tb.control_manager.write(0x0014, encode([0x0],4))
+    await tb.control_manager.write(0x0018, encode([0x0],4))
+    await tb.control_manager.write(0x001C, encode([0x0],4))
+    await tb.control_manager.write(0x0020, encode([0x0],4))
+    await tb.control_manager.write(0x0024, encode([0x0],4))
+    #Assert ap_start by writing 1 to 0x0000
+    await tb.control_manager.write(0x0000, encode([0x1],1))


TODO: Change this from being hardcoded to be dynamic and also dependent on if we are targeting Xilinx or not

nathanielnrn · 2024-08-20T06:00:14Z

yxi/xml/xml_generator.py

@@ -23,7 +23,7 @@ def gen_xml(yxi):
        root,
        "kernel",
        {
-            "name": yxi["toplevel"],
+            "name": "wrapper",


TODO: Might need to change this from wrapper to Toplevel according to the backwards compatibility fix implemented, mentioned above.

…ith verilog AXI wrapper

rachitnigam · 2024-10-09T19:53:57Z

@sampsyo @nathanielnrn what should we do with this PR? I assume @nathanielnrn has massively less time to directly work on this so we should make a decision about how we're going to merge things or close them out.

nathanielnrn and others added 30 commits June 19, 2024 17:18

add case statement support for ComponentBuilders

253dfac

add runt tests

64edeb2

make channels for the subordinate controller

0798490

WIP: read controller. All channels exist

dbb5bd4

change case to return a par block instead of automatically adding to …

94e19ec

…control of a component

Merge branch 'case-statements' into xilinx-subordinate

669febd

WIP AXI subordinate controller

3d0b022

TODO: Hook up the slices in the highest level module for ap_done, ap_start. Also thinnk about go done signals/how these connect to rest of wrapper

Queues: use case statements in FIFO control (#2171)

5f65833

* Prepare fifo for case idiom * Attempt with . Failing due to empty control * Delete calyx-py/test/correctness/queues/fifo.futil

improve naming of equalities and add name functions to some classes i…

ea71a0a

…n builder

runt tests

1ba2147

Merge branch 'case-statements' of github.com:calyxir/calyx into case-…

8e16087

…statements

apply black

98f3842

Merge branch 'main' into case-statements

03e3a14

Merge branch 'case-statements' into xilinx-subordinate

7028a82

remove extra print

82acb17

runt tests

6c2ced1

Merge branch 'main' into case-statements

c2dc415

Merge branch 'case-statements' into xilinx-subordinate

c56a5a9

print out signals when possible for case statement signals

c968a9e

Merge branch 'case-statements' of github.com:calyxir/calyx into case-…

a8f2ac6

…statements

Merge branch 'case-statements' into xilinx-subordinate

8bdd84c

might have working axi-controller. TODO: hoko up with overall wrapper

0ec3e33

remove unused function from port attribute PR

9b2740a

Controller is largely done

99f215a

TODO: Hook up ap_start from controller in wrapper to the main_compute module

work on controller and hook up to dynamic axi

590bdef

generator updates wip

b9b23d4

fud2 update variable names

8d98db2

Add init invokes to ar,aw and w channels

0e38f1f

This works around #2198 causing combinational loops

AddAXI manager to axi_test and writing to the control register

adfdfa6

WIP do mnaual ref hoisting. Problem with signal names

298fe12

nathanielnrn and others added 16 commits August 1, 2024 09:06

wip get verilog to xo fud2 invocation working. TODO: Figure out why g…

b9a2a50

…lob does not find any .v files

get vitis invocation to start correctly

9dcbc14

change kernel root to expect 'wrapper' toplevel name instead of 'Topl…

996aadc

…evel'. This generator will not work with the old verilog-wrapper

change gen_xo.tcl associate_bus_interface clock from 'ap_clk' to 'clk'

5d4f2d0

add a todo

9eac833

remove stray merge conflict from lib.rs

058c259

channel reset to be driven by ap_rst_n

7d9cf0d

Merge branch 'xilinx-subordinate' into xml-fud2

2dc2b95

xclrun: Remove timeout parameter

adbf430

This is no longer an argument to our YXI-wrapped kernels; it only exists in the old Verilog-generator stuff. We maybe want to revert/make this optional at some point to make `xclrun` compatible with the old version too?

Merge branch 'xml-fud2' of github.com:calyxir/calyx into xml-fud2

366b5b2

Merge branch 'main' into xilinx-subordinate

c065d84

Merge branch 'xilinx-subordinate' into xml-fud2

40bd016

update dynamic wrapper to use an ap_clk signal

ea36a0e

Change axi wrapper output to maintain compatability

4e98348

This allows us to not change the `kernel.xml` we use in the old wrapper vs the new wrapper and still get correct results

Merge branch 'main' into limited-ref-ports

e9e9ff2

Merge branch 'limited-ref-ports' into xml-fud2

5bd65e8

nathanielnrn commented Aug 20, 2024

View reviewed changes

fud2/scripts/xilinx.rhai Outdated

Copy link

Contributor Author

nathanielnrn Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the fud2 changes

nathanielnrn commented Aug 20, 2024

View reviewed changes

nathanielnrn marked this pull request as draft August 20, 2024 06:00

nathanielnrn added 5 commits August 20, 2024 12:09

rename generated toplevel to 'Toplevel' for backwards compatibility w…

df39ae9

…ith verilog AXI wrapper

make cocotb testing dynamic w.r.t memory base address control registers

442b1ab

add calyx wrapper flag for calyx wrapped AXI runnign via xclrun

f4fc022

builder cleanup

9b14742

rename wrapper to 'Toplevel'

561a812

Base automatically changed from limited-ref-ports to main August 28, 2024 16:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Create a backwards compatible flow for `xo`/`xclbin` generation with Calyx-AXI-wrapper #2267

WIP: Create a backwards compatible flow for `xo`/`xclbin` generation with Calyx-AXI-wrapper #2267

nathanielnrn commented Aug 20, 2024 •

edited

Loading

nathanielnrn Aug 20, 2024

nathanielnrn Aug 20, 2024

nathanielnrn Aug 20, 2024

nathanielnrn Aug 20, 2024

nathanielnrn Aug 20, 2024

nathanielnrn Aug 20, 2024

rachitnigam commented Oct 9, 2024

WIP: Create a backwards compatible flow for xo/xclbin generation with Calyx-AXI-wrapper #2267

Are you sure you want to change the base?

WIP: Create a backwards compatible flow for xo/xclbin generation with Calyx-AXI-wrapper #2267

Conversation

nathanielnrn commented Aug 20, 2024 • edited Loading

EDIT: More work than I thought regarding backwards compatibility. Commented where changes are needed

nathanielnrn Aug 20, 2024

Choose a reason for hiding this comment

nathanielnrn Aug 20, 2024

Choose a reason for hiding this comment

nathanielnrn Aug 20, 2024

Choose a reason for hiding this comment

nathanielnrn Aug 20, 2024

Choose a reason for hiding this comment

nathanielnrn Aug 20, 2024

Choose a reason for hiding this comment

nathanielnrn Aug 20, 2024

Choose a reason for hiding this comment

rachitnigam commented Oct 9, 2024

WIP: Create a backwards compatible flow for `xo`/`xclbin` generation with Calyx-AXI-wrapper #2267

WIP: Create a backwards compatible flow for `xo`/`xclbin` generation with Calyx-AXI-wrapper #2267

nathanielnrn commented Aug 20, 2024 •

edited

Loading