-
Notifications
You must be signed in to change notification settings - Fork 902
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
techmap: Add a Kogge-Stone option for $lcu
mapping
#4298
Conversation
You use this with |
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! |
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
It wouldn't work exactly this way, since the rule priority is derived from the name of the rule (e.g. |
Nice, that's good to hear!
Ah got it, I think I misunderstood the rule priority determination. Thanks for the clarification. |
@@ -42,7 +42,7 @@ module _80_lcu_kogge_stone (P, G, CI, CO); | |||
g[0] = g[0] | (p[0] & CI); | |||
|
|||
for (i = 0; i < $clog2(WIDTH); i = i + 1) begin | |||
for (j = 2**i; j < WIDTH; j = j + 1) begin | |||
for (j = WIDTH - 1; j >= 2**i; j = j - 1) begin |
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.
Surprisingly enough, this passed equivalence testing before the fix
No description provided.