-
Notifications
You must be signed in to change notification settings - Fork 52
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
Change the simulation harness to not use readmemh
and writememh
#1603
Comments
Hmm; this is an interesting point. Here are two very tentative thoughts, which are kind of connected:
Anyway, it's definitely a good idea to be able test the "real" external interface in the way you describe! |
So, I thought our AXI tests already do (2) since the AXI wrapper generator needs to pass in memories by reference |
Ah, taking a glance at the generated Verilog, I think you're right. (For some reason, I had it in my head that it was replacing the externalized memories with special AXI-ified memories; not sure where I got that idea.) It might be good idea to assess whether testing this AXI wrapper would suffice to test what we want… (not that a simple non-AXI test harness for the externalized interface wouldn't be a good idea too). |
Okay, here is an actionable approach to this:
This way, we have created a clear interface for what the Calyx compiler is responsible for and what the "simulation wrapper generator" is responsible for. This will also allow us to explain the existence of |
This is a nice idea. To expand on this a tiny bit:
|
@ayakayorihiro's efforts on a FIRRTL backend for Calyx (cf #1898, most recently) have brought up this issue again, and crystallized a bit more what we need to do here. Specifically, I think this is what we should probably do at some point: Deprecate
|
Awesome, I support getting rid of all of this cruft ( |
Just a little something to chime in on, migrating from Also, one thing to note is that programs using |
We've had ongoing problems with really understanding how to handle
@external
cells and their ports. For example, #1034 tracks a slew of issues where we either keep attempting to add back@clk
and@reset
ports from these memories or remove them because they behave incorrectly for the synthesis backend.I think the right thing to do here is completely getting rid of the magical
readmemh
andwritememh
functions we emit for the simulation backend and forcing ourselves to use the externalized signature of the top-level component directly.Concretely, this would mean:
externalize
pass runs for the default compilation pipelineI think this will provide clarity on issues like #1034 as well since it'll force us to use our designs in a that our users are having to.
The text was updated successfully, but these errors were encountered: