search

calyxir / calyx

Intermediate Language (IL) for Hardware Accelerator Generators
https://calyxir.org
MIT License
450 stars 44 forks source link

Add bypass register primitive #2030

Open matth2k opened 2 weeks ago

matth2k commented 2 weeks ago

Having this primitive built into the Calyx library would be useful for the AMC memory flow. The use case for this is I have an arbiter de-muxing data from a memory load interface, so I introduced my own inline primitive to latch the data without adding a cycle of latency.

Are there any test cases I should add, besides fixing the ones it broke? Either for parsing sake or an actual use-case? Should the stable attribute on the bypass register be removed?

andrewb1999 commented 2 weeks ago

I'm not sure that compile.futil is the right place for this primitive since it's not required to compile Calyx programs to verilog. I agree it would be nice to have this somewhere in the primitive library though, I think this pattern will start showing up a lot in dynamic use cases. @rachitnigam are you okay with this being somewhere in the primitive library? And if so, where should it go?

andrewb1999 commented 2 weeks ago

This is failing some test cases and I'm not really sure why.

rachitnigam commented 2 weeks ago

Hey @matth2k! Thanks for opening the PR. @andrewb1999 is right: we don't want this in compile.futil since that is packaged into every Calyx file for compilation. I would recommend adding this to core.futil and core.sv instead.

Also, we should definitely add a test to make sure the primitive works. No other frontend uses it so very likely that a change could break it in the future.