-
Notifications
You must be signed in to change notification settings - Fork 25
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
csr.bus: redesign Multiplexer shadow registers. #41
Conversation
3846a9b
to
d778b3d
Compare
That's amazing! I expected the savings to be good but I didn't expect them to be this good. |
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 is very nicely and cleanly implemented. Great job!
Before this commit, csr.Multiplexer had separate shadows for every element in its memory map. The same shadow was shared for read and write accesses to an element; a combined read/write transaction was impossible despite being allowed by the CSR interface. After this commit, csr.Multiplexer has separate shadows for read and write accesses, but both shadows are shared by every element using them. For multiplexers with many elements, this approach also results in significant resource savings.
d778b3d
to
39194ac
Compare
Merged, at last! Thank you @whitequark for providing the idea behind this redesign, and for reviewing it. |
Happy to have brought this to completion! This will be extremely useful. |
Before this commit,
csr.Multiplexer
had separate shadows for every element in its memory map. The same shadow was shared for read and write accesses to anElement
; a combined read/write transaction was impossible despite being allowed by the CSR interface.After this commit,
csr.Multiplexer
has separate shadows for read and write accesses, but both shadows are shared by everyElement
using them. For multiplexers with many elements, this approach also results in significant resource savings.Example
To measure the resource savings, let's consider this example:
Statistics
yosys -p "synth_ecp5 -nowidelut"
would report this:shadow_overlaps=None
,yosys -p "synth_ecp5 -nowidelut"
reports this:The new design uses 2x less LUT4s and 7x less FFs !
shadow_overlaps=3
,yosys -p "synth_ecp5 -nowidelut"
reports this:The
shadow_overlaps
parameter is exposed to users in case it proves useful, although LUT packing benefits seem to be outweighed by the increased cost in address decoding logic.None
seem to be a good default for FPGAs, at least.In these three cases, running
nextpnr-ecp5 --out-of-context
(or--pack-only
) afterwards reports the same amount of LUTs/FFs.