-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[SCEV] Fold ((-1 * C1) * D / C1) -> -1 * D. #157555
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: Florian Hahn (fhahn) ChangesTreat negative constants C as -1 * abs(C1) when folding multiplies and udivs. Alive2 Proof: https://alive2.llvm.org/ce/z/bdj9W2 Full diff: https://github.com/llvm/llvm-project/pull/157555.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index bd57d1192eb94..93f20bedc75e3 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -3219,10 +3219,15 @@ const SCEV *ScalarEvolution::getMulExpr(SmallVectorImpl<const SCEV *> &Ops,
// Try to fold (C * D /u C) -> D, if C is a power-of-2 and D is a multiple
// of C.
const SCEV *D;
- if (match(Ops[1], m_scev_UDiv(m_SCEV(D), m_scev_Specific(LHSC))) &&
- LHSC->getAPInt().isPowerOf2() &&
- LHSC->getAPInt().logBase2() <= getMinTrailingZeros(D)) {
- return D;
+ APInt LHSV = LHSC->getAPInt();
+ auto *C2 =
+ cast<SCEVConstant>(LHSV.isNegative() && !LHSV.isMinSignedValue()
+ ? getAbsExpr(LHSC, false)
+ : LHSC);
+ if (C2 && match(Ops[1], m_scev_UDiv(m_SCEV(D), m_scev_Specific(C2))) &&
+ C2->getAPInt().isPowerOf2() &&
+ C2->getAPInt().logBase2() <= getMinTrailingZeros(D)) {
+ return C2 == LHSC ? D : getNegativeSCEV(D);
}
}
}
diff --git a/llvm/test/Analysis/ScalarEvolution/mul-udiv-folds.ll b/llvm/test/Analysis/ScalarEvolution/mul-udiv-folds.ll
index b5d976ba3acda..9f5ed91fbec10 100644
--- a/llvm/test/Analysis/ScalarEvolution/mul-udiv-folds.ll
+++ b/llvm/test/Analysis/ScalarEvolution/mul-udiv-folds.ll
@@ -23,7 +23,7 @@ define void @udiv4_and_udiv2(i1 %c, ptr %A) {
; CHECK-NEXT: %gep.16 = getelementptr i16, ptr %A, i64 %iv
; CHECK-NEXT: --> {((2 * ((zext i32 %start to i64) /u 4))<nuw><nsw> + %A),+,2}<%loop> U: full-set S: full-set Exits: ((zext i32 %start to i64) + %A) LoopDispositions: { %loop: Computable }
; CHECK-NEXT: %gep.32 = getelementptr i32, ptr %A, i64 %iv
-; CHECK-NEXT: --> {((zext i32 %start to i64) + %A),+,4}<%loop> U: full-set S: full-set Exits: ((zext i32 %start to i64) + (4 * ((zext i32 %start to i64) /u 2))<nuw><nsw> + (-4 * ((zext i32 %start to i64) /u 4))<nsw> + %A) LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: --> {((zext i32 %start to i64) + %A),+,4}<%loop> U: full-set S: full-set Exits: ((4 * ((zext i32 %start to i64) /u 2))<nuw><nsw> + %A) LoopDispositions: { %loop: Computable }
; CHECK-NEXT: %gep.40 = getelementptr <{ i32, i8 }>, ptr %A, i64 %iv
; CHECK-NEXT: --> {((5 * ((zext i32 %start to i64) /u 4))<nuw><nsw> + %A),+,5}<%loop> U: full-set S: full-set Exits: ((5 * ((zext i32 %start to i64) /u 2))<nuw><nsw> + %A) LoopDispositions: { %loop: Computable }
; CHECK-NEXT: %gep.48 = getelementptr <{ i32, i16 }>, ptr %A, i64 %iv
|
Treat negative constants C as -1 * abs(C1) when folding multiplies and udivs. Alive2 Proof: https://alive2.llvm.org/ce/z/bdj9W2
ec99aa1
to
b635047
Compare
// D is a multiple of C2, and C1 is a multiple of C2. | ||
const SCEV *D; | ||
APInt C1V = LHSC->getAPInt(); | ||
// If C1 is negative, try (-1 * abs(C1)) instead. |
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.
Adjust the comment to something like:
(C1 * D /u C2) == -1 * -C1 * D /u C2 when C1 != INT_MIN
Though maybe that's not quite right because that equality still holds (just not in a useful way) when C1 is INT_MIN.
Basically, we need something here with more of indication of why it's correct.
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.
Updated, thanks!
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 w/prior comment addressed.
const SCEV *D; | ||
APInt C1V = LHSC->getAPInt(); | ||
// (C1 * D /u C2) == -1 * -C1 * D /u C2 when C1 != INT_MIN. | ||
if (C1V.isNegative() && !C1V.isMinSignedValue()) |
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.
Why is the INT_MIN check here needed? Isn't abs() just a no-op for 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.
Ah, I guess this avoid doing an unnecessary getNegativeSCEV() lateron. No, that's based on the equality comparison, so it shouldn't matter if we do an extra abs() here.
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.
Yeah it's a no-op, we would end up with (-INT_MIN * D /u C2) == (-1 * -INT_MIN * D /u C2)
. Can change to use abs()
if you prefer and remove the check?
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.
I'll leave it as-is for now, when this gets generalized to non-constants we can adjust it
Treat negative constants C as -1 * abs(C1) when folding multiplies and udivs. Alive2 Proof: https://alive2.llvm.org/ce/z/bdj9W2 PR: llvm/llvm-project#157555
If C2 >u C1 and C1 >u 1, fold to A /u (C2 /u C1). Depends on llvm#157555. Alive2 Proof: https://alive2.llvm.org/ce/z/BWvQYN
If C2 >u C1 and C1 >u 1, fold to A /u (C2 /u C1). Depends on #157555. Alive2 Proof: https://alive2.llvm.org/ce/z/BWvQYN PR: #157656
… (#157656) If C2 >u C1 and C1 >u 1, fold to A /u (C2 /u C1). Depends on llvm/llvm-project#157555. Alive2 Proof: https://alive2.llvm.org/ce/z/BWvQYN PR: llvm/llvm-project#157656
…158328) This reverts commit fd58f23. The recommit contains an extra check to make sure that D is a multiple of C2, if C2 > C1. This fixes the issue causing the revert fd58f23. Tests have been added in 6a726e9. Original message: If C2 >u C1 and C1 >u 1, fold to A /u (C2 /u C1). Depends on #157555. Alive2 Proof: https://alive2.llvm.org/ce/z/BWvQYN PR: #157656
… C2 > C1." (#158328) This reverts commit fd58f23. The recommit contains an extra check to make sure that D is a multiple of C2, if C2 > C1. This fixes the issue causing the revert fd58f23. Tests have been added in 6a726e9. Original message: If C2 >u C1 and C1 >u 1, fold to A /u (C2 /u C1). Depends on llvm/llvm-project#157555. Alive2 Proof: https://alive2.llvm.org/ce/z/BWvQYN PR: llvm/llvm-project#157656
Treat negative constants C as -1 * abs(C1) when folding multiplies and udivs.
Alive2 Proof: https://alive2.llvm.org/ce/z/bdj9W2