[Comb] Expand Known-Bits Analysis to Additional Operators #8902
Conversation
cowardsa
changed the title
uenoku
left a comment
uenoku
left a comment
There was a problem hiding this comment.
LGTM. We need unit tests for this but I don't block the PR as there hasn't been a test for other operations as well.
| // `shl(x, y)` is the known bits of `x` << known bits of `y`. | ||
| if (auto shlOp = dyn_cast<ShlOp>(op)) { | ||
| auto lhs = computeKnownBits(shlOp.getOperand(0), depth + 1); | ||
| auto rhs = computeKnownBits(shlOp.getOperand(1), depth + 1); | ||
| auto res = KnownBits::shl(lhs, rhs); | ||
| return res; | ||
| } | ||
|
|
||
| // `shr(x, y)` is the known bits of `x` >> known bits of `y`. | ||
| if (auto shrOp = dyn_cast<ShrUOp>(op)) { | ||
| auto lhs = computeKnownBits(shrOp.getOperand(0), depth + 1); | ||
| auto rhs = computeKnownBits(shrOp.getOperand(1), depth + 1); | ||
| auto res = KnownBits::lshr(lhs, rhs); | ||
| return res; | ||
| } |
There was a problem hiding this comment.
Cool, didn't know that this smart shift API for KnownBits exists.
|
@uenoku - sorry meant to add a question in the description. Don't think we have any existing KnownBits tests - any thoughts on where such tests should go? Would it require a test pass to be written? |
|
I think we can create a unite test folder for Comb (here is for AIG and probably we can put IR and query knowBits analysis to them ) but test pass also makes sense to me |
|
@uenoku and @fabianschuiki - added KnownBits tests - let me know what you think? Wanted to add them now as worth checking things are behaving as expected! |
uenoku
left a comment
uenoku
left a comment
There was a problem hiding this comment.
Looks awesome, thank you for adding tests!
2 participants
Add support for extract, and logical shift operators to KnownBits analysis - helping to improve QoR for synth results. A future PR may aim to improve performance by invoking a caching mechanism that uses the MLIR analysis runners.