-
Notifications
You must be signed in to change notification settings - Fork 777
OptimizeInstruction: Optimize any boolean & (No overlap with boolean's LSB) ==> 0 #7505
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
base: main
Are you sure you want to change the base?
Changes from all commits
b67c4ac
340fd1b
6aab029
11992ac
bfdc24c
27ac17f
2f2dbfb
a8216a0
2bbd847
dad3fd5
51196f8
131a423
0f09450
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -837,6 +837,9 @@ struct OptimizeInstructions | |||||
if (auto* ret = combineAnd(curr)) { | ||||||
return replaceCurrent(ret); | ||||||
} | ||||||
if (auto* ret = optimizeAndNoOverlappingBits(curr)) { | ||||||
return replaceCurrent(ret); | ||||||
} | ||||||
} | ||||||
// for or, we can potentially combine | ||||||
if (curr->op == OrInt32) { | ||||||
|
@@ -850,6 +853,12 @@ struct OptimizeInstructions | |||||
return replaceCurrent(ret); | ||||||
} | ||||||
} | ||||||
if (curr->op == AndInt64) { | ||||||
if (auto* ret = optimizeAndNoOverlappingBits(curr)) { | ||||||
return replaceCurrent(ret); | ||||||
} | ||||||
} | ||||||
|
||||||
// relation/comparisons allow for math optimizations | ||||||
if (curr->isRelational()) { | ||||||
if (auto* ret = optimizeRelational(curr)) { | ||||||
|
@@ -3549,6 +3558,52 @@ struct OptimizeInstructions | |||||
return nullptr; | ||||||
} | ||||||
|
||||||
// Bitwise AND of a value with bits in [0, n) and a constant with no bits in | ||||||
// [0, n) always yields 0. Replace with zero. | ||||||
Expression* optimizeAndNoOverlappingBits(Binary* curr) { | ||||||
assert(curr->op == AndInt32 || curr->op == AndInt64); | ||||||
|
||||||
using namespace Abstract; | ||||||
using namespace Match; | ||||||
|
||||||
auto* left = curr->left; | ||||||
auto* right = curr->right; | ||||||
|
||||||
// Check right as constant and left's max bits | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
auto leftMaxBits = Bits::getMaxBits(left, this); | ||||||
uint64_t maskLeft; | ||||||
if (leftMaxBits == 64) { | ||||||
maskLeft = 0xffffffffffffffffULL; // All bits set for 64-bit case | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can just |
||||||
} else { | ||||||
maskLeft = (1ULL << leftMaxBits) - 1; | ||||||
} | ||||||
if (auto* c = right->dynCast<Const>()) { | ||||||
uint64_t constantValue = c->value.getInteger(); | ||||||
if ((constantValue & maskLeft) == 0) { | ||||||
return getDroppedChildrenAndAppend( | ||||||
curr, LiteralUtils::makeZero(curr->left->type, *getModule())); | ||||||
} | ||||||
} | ||||||
|
||||||
// Check left as constant and right's max bits | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this should not be needed. We canonicalize code so that constants are on the right, in part to help writing optimizations like this. So you can assume the constant is on the right. (And let me know if you see a case where that is not true - it is a bug.) |
||||||
auto rightMaxBits = Bits::getMaxBits(right, this); | ||||||
uint64_t maskRight; | ||||||
if (rightMaxBits == 64) { | ||||||
maskRight = 0xffffffffffffffffULL; // All bits set for 64-bit case | ||||||
} else { | ||||||
maskRight = (1ULL << rightMaxBits) - 1; | ||||||
} | ||||||
if (auto* c = left->dynCast<Const>()) { | ||||||
uint64_t constantValue = c->value.getInteger(); | ||||||
if ((constantValue & maskRight) == 0) { | ||||||
return getDroppedChildrenAndAppend( | ||||||
curr, LiteralUtils::makeZero(curr->right->type, *getModule())); | ||||||
} | ||||||
} | ||||||
|
||||||
return nullptr; | ||||||
kripken marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
// We can combine `or` operations, e.g. | ||||||
// (x > y) | (x == y) ==> x >= y | ||||||
// (x != 0) | (y != 0) ==> (x | y) != 0 | ||||||
|
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.
The comment here is in the opposite order of the code. I would reverse it, to follow the code.