-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[ConstraintElim] Remove dead code. NFC. #118983
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
|
@llvm/pr-subscribers-llvm-analysis Author: Yingwei Zheng (dtcxzyw) Changes
Full diff: https://github.com/llvm/llvm-project/pull/118983.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/ConstraintSystem.cpp b/llvm/lib/Analysis/ConstraintSystem.cpp
index e4c9dcc7544e99..7216a0219080f4 100644
--- a/llvm/lib/Analysis/ConstraintSystem.cpp
+++ b/llvm/lib/Analysis/ConstraintSystem.cpp
@@ -52,9 +52,6 @@ bool ConstraintSystem::eliminateUsingFM() {
for (unsigned R1 = 0; R1 < NumRemainingConstraints; R1++) {
// FIXME do not use copy
for (unsigned R2 = R1 + 1; R2 < NumRemainingConstraints; R2++) {
- if (R1 == R2)
- continue;
-
int64_t UpperLast = getLastCoefficient(RemainingRows[R2], LastIdx);
int64_t LowerLast = getLastCoefficient(RemainingRows[R1], LastIdx);
assert(
|
| if (R1 == R2) | ||
| continue; |
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.
might be worth adding an assert initially?
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.
It is safe to remove this because R1 and R2 are both not modified inside the loop.
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.
Ah yes, thanks for the cleanup!
nikic
left a comment
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
fhahn
left a comment
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, thanks for the cleanup!
| if (R1 == R2) | ||
| continue; |
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.
Ah yes, thanks for the cleanup!
R2should be always greater thanR1here.Code coverage: https://dtcxzyw.github.io/llvm-opt-benchmark/coverage/data/zyw/opt-ci/actions-runner/_work/llvm-opt-benchmark/llvm-opt-benchmark/llvm/llvm-project/llvm/lib/Analysis/ConstraintSystem.cpp.html#L56