-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[DA] Remove special handling for SCEVAddExpr in GCD MIV #169927
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: users/kasuga-fj/da-gcdmiv-delta-overflow-test
Are you sure you want to change the base?
[DA] Remove special handling for SCEVAddExpr in GCD MIV #169927
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-llvm-analysis Author: Ryotaro Kasuga (kasuga-fj) ChangesIn Fix one of the tests added in #169926. Full diff: https://github.com/llvm/llvm-project/pull/169927.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 77b09fb15316e..bb54c0986c20f 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -2590,23 +2590,6 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
const SCEV *Delta = SE->getMinusSCEV(DstConst, SrcConst);
LLVM_DEBUG(dbgs() << " Delta = " << *Delta << "\n");
const SCEVConstant *Constant = dyn_cast<SCEVConstant>(Delta);
- if (const SCEVAddExpr *Sum = dyn_cast<SCEVAddExpr>(Delta)) {
- // If Delta is a sum of products, we may be able to make further progress.
- for (const SCEV *Operand : Sum->operands()) {
- if (isa<SCEVConstant>(Operand)) {
- assert(!Constant && "Surprised to find multiple constants");
- Constant = cast<SCEVConstant>(Operand);
- } else if (const SCEVMulExpr *Product = dyn_cast<SCEVMulExpr>(Operand)) {
- // Search for constant operand to participate in GCD;
- // If none found; return false.
- std::optional<APInt> ConstOp = getConstanCoefficient(Product);
- if (!ConstOp)
- return false;
- ExtraGCD = APIntOps::GreatestCommonDivisor(ExtraGCD, ConstOp->abs());
- } else
- return false;
- }
- }
if (!Constant)
return false;
APInt ConstDelta = cast<SCEVConstant>(Constant)->getAPInt();
diff --git a/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
index 618d14c75faad..f6b23f18df738 100644
--- a/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
@@ -127,9 +127,8 @@ exit:
; A[-3*i - 3*m - INT64_MAX] = 1;
; }
;
-; FIXME: DependenceAnalysis currently detects no dependency between the two
-; stores, but it may exist. For example, consider `m = 1`. Then,
-; `-3*m - INT64_MAX` is `INT64_MAX - 1`. So `-3*i - 3*m - INT64_MAX` is
+; Dependency may exist between the two stores. For example, consider `m = 1`.
+; Then, `-3*m - INT64_MAX` is `INT64_MAX - 1`. So `-3*i - 3*m - INT64_MAX` is
; effectively `-3*i + (INT64_MAX - 1)`. Thus, accesses will be as follows:
;
; memory access | i == 1 | i == max_i - 1
@@ -143,7 +142,7 @@ define void @gcdmiv_const_ovfl(ptr %A, i64 %m) {
; CHECK-ALL-NEXT: Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 0, ptr %gep.0, align 1
; CHECK-ALL-NEXT: da analyze - none!
; CHECK-ALL-NEXT: Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
-; CHECK-ALL-NEXT: da analyze - none!
+; CHECK-ALL-NEXT: da analyze - output [*|<]!
; CHECK-ALL-NEXT: Src: store i8 1, ptr %gep.1, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
; CHECK-ALL-NEXT: da analyze - none!
;
@@ -151,7 +150,7 @@ define void @gcdmiv_const_ovfl(ptr %A, i64 %m) {
; CHECK-GCD-MIV-NEXT: Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 0, ptr %gep.0, align 1
; CHECK-GCD-MIV-NEXT: da analyze - consistent output [*]!
; CHECK-GCD-MIV-NEXT: Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
-; CHECK-GCD-MIV-NEXT: da analyze - none!
+; CHECK-GCD-MIV-NEXT: da analyze - consistent output [*|<]!
; CHECK-GCD-MIV-NEXT: Src: store i8 1, ptr %gep.1, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
; CHECK-GCD-MIV-NEXT: da analyze - consistent output [*]!
;
|

In
gcdMIVtest, there is logic that assumes the addition(s) ofSCEVAddExprdon't overflow without any checks. Adding overflow checks would be fine, but this part appeart to be less useful. So this patch removes it.Fix one of the tests added in #169926.