-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce verilog -> cocotb simulation fud2
path
#1997
base: main
Are you sure you want to change the base?
Conversation
* Sketch an example fud2 op * Fix typo --------- Co-authored-by: Adrian Sampson <adrian@radbox.org>
Testing locally on vec add, the hard-coded cocotb tests pass, meaning there is parity with the previous bash scripts
Makefile expects a DATA_PATH and VERILOG_SOURCE
cocotb expects datapath and relatviepath to be relative to the actual Makefile, not from the invocation
80c5644
to
5ba23e1
Compare
078a015
to
f2ba9c3
Compare
This reverts commit 09ac9c2.
I think this is finally ready for review if someone gets the chance. I know #2084 has some implications on |
Yeah definitely merge this first. We're not quite ready to migrate things yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent! This all looks great! Here are just a few minor comments.
@@ -174,7 +174,7 @@ jobs: | |||
uses: actions-rs/cargo@v1 | |||
with: | |||
command: build | |||
args: --all-features --manifest-path /home/calyx/Cargo.toml | |||
args: --all --all-features --manifest-path /home/calyx/Cargo.toml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two tiny things: cargo build --help
suggests that --all
is deprecated in favor of --workspace
, and just wanted to make sure that we actually do want to build everything in the workspace.
//Get yxi file from main compute program. | ||
//TODO(nate): Can this use the `yxi` operation instead of hardcoding the build cmd calyx rule with arguments? | ||
// Get yxi file from main compute program. | ||
// TODO(nate): Can this use the `yxi` operation instead of hardcoding the build cmd calyx rule with arguments? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will probably have to wait for #1958.
// Cocotb is wants files relative to the location of the makefile. | ||
// This is annoying to calculate on the fly, so we just copy necessary files to the build directory | ||
e.rule("copy", "cp $in $out")?; | ||
e.rule("make", "make DATA_PATH=$sim_data VERILOG_SOURCE=$in COCOTB_LOG_LEVEL=CRITICAL > $out")?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this rule should be called make-cocotb
or something (instead of the more generic make
)?
Introduces a path accepting
.v
files, presumably axi-wrapped via afud2 foo.futil --through axi-wrapped
and outputting the final contents of AXI rams simulated viacocotb.
Invocation currently has to be manually specified with
--through
due to the behavior ofguess_state
An invocation looks like
fud2 <path to axi wrapped verilog> --from verilog-noverify --to cocotb-axi --set sim.data=<path to .data/json file>
This PR also introduces some
cocotb
python runners that largely looks like the xilinx cocotb harness. Those will hopefully be removed once we get the wrapper to properly interface with XRT.Adds some
runt
tests as well that check the state of some AXI generation intermediate steps (including simulation output with cocotb). I think that the main changes of note besides that is thiscocotb
python runner mentioned above and thefud2
changes