search

YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.31k stars 860 forks source link

techmap: Add a Kogge-Stone option for `$lcu` mapping #4298

Closed povik closed 3 months ago

povik commented 3 months ago

You use this with techmap -D KOGGE_STONE, alternatively we can put this into a file of its own as a rule that takes precedence over the default rule, so one would do techmap -map +/techmap.v -map +/kogge-stone.v or similar.

jamestiotio commented 3 months ago

Hi @povik, I hope you don't mind me chiming in here from a user perspective. The "Kogge-Stone" in the PR title caught my attention. 😄

Putting this into a file of its own would be neater in my opinion. This is mainly because there are various "flavors" of LCUs and tree adders, each with its own benefits and limitations (I assume Kogge-Stone and Brent-Kung are the most popular ones). Allowing users to select different LCU variations by specifying the file that they want seems to be more organized.

This would also allow us to add more variations more easily in the future. We can simply add more files instead of having to fiddle around with multiple conditional directives. This also has the benefit of not having to worry about the order of these directives if the user is the one who specifies the rule precedence.

Just some things to consider for this implementation. Thanks for the great work!

povik commented 3 months ago

Hi @jamestiotio, not at all, feel free to chime in. We have discussed this among Yosys maintainers after I opened the PR and the plan now is to go with separate files, like you suggest. So this will get renamed to +/choices/kogge-stone.v. Among other points this has the advantage that one can do synth -extra-map +/choices/kogge-stone.v.

This also has the benefit of not having to worry about the order of these directives if the user is the one who specifies the rule precedence.

It wouldn't work exactly this way, since the rule priority is derived from the name of the rule (e.g. _90_lcu), not the file it's placed in, or the position of that file on the argument list to techmap.

jamestiotio commented 3 months ago

We have discussed this among Yosys maintainers after I opened the PR and the plan now is to go with separate files, like you suggest.

Nice, that's good to hear!

It wouldn't work exactly this way, since the rule priority is derived from the name of the rule (e.g. _90_lcu), not the file it's placed in, or the position of that file on the argument list to techmap.

Ah got it, I think I misunderstood the rule priority determination. Thanks for the clarification.