Skip to content
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

[CombFolds] Don't canonicalize extract(shl(1, x)) if shift is multiply used #7527

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

uenoku
Copy link
Member

@uenoku uenoku commented Aug 19, 2024

There is a canonicalization for exract(c, shl(1, x)) to x == c but this canonicalization introduces a bunch of comparision to constants. This harms PPA when bitwidth is large (e.g. 16 bit shift introduce 2^16 icmp op). To prevent such regressions this commit imposes restriction regarding the number of uses for shift.

…y used

There is a canonicalization for `exract(c, shl(1, x))` to `x == c` but this
canonicalization introduces a bunch of comparision to constants. This harms
PPA when bitwidth is large (e.g. 16 bit shift introduce 2^16 icmp op). To prevent
such regressions this commit imposes restriction regarding the number of uses
for shift.
@uenoku uenoku requested a review from darthscsi as a code owner August 19, 2024 07:42
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM barring a diff of an internal design to see how much this changes.

@uenoku uenoku merged commit 5d8cf69 into main Aug 21, 2024
4 checks passed
@uenoku uenoku deleted the dev/hidetou/comb-fold branch August 21, 2024 03:25
@uenoku
Copy link
Member Author

uenoku commented Aug 21, 2024

There are substantial changes in output verilog but all of them are expected changes. The change should preserve a design intent of one-hot encoding in a much better way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants