-
Couldn't load subscription status.
- Fork 383
[CombToSynth] Compute Kogge-Stone prefix tree lazily in unsigned comparison lowering #9050
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
Conversation
6c9ce16 to
ec10b65
Compare
…arison lowering Add LazyKoggeStonePrefixTree class for on-demand computation of prefix values, use lazy evaluation in ICmpOp conversion to avoid computing all intermediate prefix values.
ec10b65 to
a7acda3
Compare
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.
LGTM - nice changes computing Brent-Kung lazily may be a bit more complex as it has backwards+forwards phases... But this one looks ideal - only minor comments
| if (it != prefixCache.end()) | ||
| return it->second; | ||
|
|
||
| assert(level > 0 && "level must be positive"); |
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.
Nit: perhaps a comment to justify that we're now moving to compute the values and we can only do that after level 0 would help - took me a minute to get that
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.
Absolutely, sorry for the confusion.
Yeah Brent-Kung would be really non-trivial and not sure if there is a clean way :) I'll probably prioritize Sklanskey as it's used in 32-bit comparison (which creates ~500 dead operations right now). |
Add LazyKoggeStonePrefixTree class for on-demand computation of prefix values, use lazy evaluation in ICmpOp conversion to avoid computing all intermediate prefix values. The class could be generalized to handle other prefix tree as well. KoggeStone is prioritized for now since it's currently used for wide comparisons (actually for smaller width it's not too bad to just rely on DCE). Locally checked LEC for larger width like i128 etc.