-
Notifications
You must be signed in to change notification settings - Fork 178
Inferring True Dual-Port BRAMs with the new IR on Series 7 and ECP5 #1011
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
Comments
@mwkmwkmwk Could you take a look, please? |
I noticed that RFC 45 was implemented in 890e099 so I thought that I'd give this another go with new I've tried my best to elaborate on either case below. I've installed Amaranth with its builtin Yosys, and I'm using Vivado v2023.1 for building for the Series 7, and Yosys 0.38+92 and nextpnr-ecp5 0.7-11-g05ed9308 for the ECP5. I've provided the full source at this gist. Successfully synthesizing a TDP RAM for the Series 7, but not the ECP5.If I use a586df8, Vivado correctly infers a TDP RAM, as seen in
And it chooses to implement it in Block Memory, as seen in
Yosys however is unable to map the memory to a TDP RAM, and instead implements everything combinationally:
This uses more resources than the ECP5 has available, so naturally nextpnr throws an error:
Successfully synthesizing a TDP RAM for the ECP5, but not the Series 7.If I use the latest stable release of Amaranth from PyPI, Vivado is unable to infer the RAM type, and produces this error before the synthesis run fails a bit later:
Yosys however has no troubles, and maps to a TDP Block RAM, and the design fits inside the ECP5:
Would be super interested in any thoughts on this. I'm also happy to attempt implementing a solution, if one is known :) Thank you all! |
This is an unfortunate necessity needed to fix memory inference regressions introduced when we switched to using v2 cells. A better approach, compatible with RFC 54, will need to be figured out for Amaranth 0.6. Fixes amaranth-lang#1011.
This is an unfortunate necessity needed to fix memory inference regressions introduced when we switched to using v2 cells. A better approach, compatible with RFC 54, will need to be figured out for Amaranth 0.6. Fixes amaranth-lang#1011.
This is an unfortunate necessity needed to fix memory inference regressions introduced when we switched to using v2 cells. A better approach, compatible with RFC 54, will need to be figured out for Amaranth 0.6. Fixes #1011.
Well done @wanda-phi! |
I'm trying to infer a true dual port BRAM on a Xilinx Series 7 chip, but I'm not having much luck. I've written the following to try to map the ReadPort/WritePort construct to the addr/din/dout/en/wea that's native to the RAM36 primitives:
And if I create a design in the following format, Vivado will recognize it as a True Dual Port BRAM. I'm building for the Nexys4DDR, which has 16 switches and 16 LEDs, and I'm using the top half of each for Port A, and the bottom half for Port B, just to prevent the memory from being optimized out.
If I build this with
Nexys4DDRPlatform().build(TrueDualPortTest())
, then Vivado will happily recognize this as a true dual port BRAM, as is seen in the logs:INFO: [Synth 8-3971] The signal "\top.tdp :/mem_reg" was recognized as a true dual port RAM template.
And the post-placement resource utilization report will show that block ram is being used. However, if I uncomment either (or both) of the two lines marked in the source above - Vivado will map everything to distributed RAM instead. This seems to hold true if I keep increasing the depth of the RAM.
I also noticed that if I assign the
en
andwe
signals synchronously instead of combinationally, Vivado will throw anUnrecognized RAM template
error.My question is: How do I create a true-dual port RAM in Amaranth? I'd like to avoid directly instantiating an inferred BRAM template, since I'm not able to use the built-in simulator on external Verilog.
Thanks everyone!
Related Info:
I did notice that a while back the Yosys memory interface was reworked and support for TDP memories was added - I'm not sure if that has any implications for Amaranth, though.
YosysHQ/yosys#1959
And that appears to be reflected in the yosys docs:
https://yosyshq.readthedocs.io/projects/yosys/en/latest/CHAPTER_Memorymap.html
The text was updated successfully, but these errors were encountered: