[llvm-branch-commits] [llvm] [DA] Stop negating Delta in the Weak Zero SIV test (PR #188212)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

<details>
<summary>Changes</summary>

This patch removes the variable `NewDelta`, which was calculated as the negation of `Delta`, along with its uses. `NewDelta` is now referenced in only one place, and that code is effectively dead because more general analysis with ConstantRange is performed at an earlier stage. Also the test using `NewDelta` is not correct when `Delta` is a signed minimum value, as negating it yields the same value as original. This patch also fixes the correctness issue in such a situation.

---
Full diff: https://github.com/llvm/llvm-project/pull/188212.diff


2 Files Affected:

- (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (-12) 
- (modified) llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-delta-signed-min.ll (+6-6) 


``````````diff
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index b51e70d94212a..db7fcefb74169 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -1825,9 +1825,6 @@ bool DependenceInfo::weakZeroSIVtestImpl(const SCEVAddRecExpr *AR,
   if (!ConstCoeff)
     return false;
 
-  const SCEV *NewDelta =
-      SE->isKnownNegative(ConstCoeff) ? SE->getNegativeSCEV(Delta) : Delta;
-
   if (const SCEV *UpperBound =
           collectUpperBound(AR->getLoop(), Delta->getType())) {
     LLVM_DEBUG(dbgs() << "\t    UpperBound = " << *UpperBound << "\n");
@@ -1847,15 +1844,6 @@ bool DependenceInfo::weakZeroSIVtestImpl(const SCEVAddRecExpr *AR,
     }
   }
 
-  // check that Delta/ARCoeff >= 0
-  // really check that NewDelta >= 0
-  if (SE->isKnownNegative(NewDelta)) {
-    // No dependence, newDelta < 0
-    ++WeakZeroSIVindependence;
-    ++WeakZeroSIVsuccesses;
-    return true;
-  }
-
   // if ARCoeff doesn't divide Delta, then no dependence
   if (isa<SCEVConstant>(Delta) &&
       !isRemainderZero(cast<SCEVConstant>(Delta), ConstCoeff)) {
diff --git a/llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-delta-signed-min.ll b/llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-delta-signed-min.ll
index 90c3477234043..01d4f218a18ec 100644
--- a/llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-delta-signed-min.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/weak-zero-siv-delta-signed-min.ll
@@ -11,14 +11,14 @@
 ;     A[-i + (1 << 62)] = 1;
 ; }
 ;
-; FIXME: There is a dependency between the two stores in all directions.
+; There is a dependency between the two stores in all directions.
 ;
 define void @weak_zero_src_siv_large_btc(ptr %A) {
 ; CHECK-ALL-LABEL: 'weak_zero_src_siv_large_btc'
 ; 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 - output [S]!
 ; 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!
 ;
@@ -26,7 +26,7 @@ define void @weak_zero_src_siv_large_btc(ptr %A) {
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 0, ptr %gep.0, align 1
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - output [S]!
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
-; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - none!
+; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - output [*|<]!
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:  Src: store i8 1, ptr %gep.1, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - output [*]!
 ;
@@ -63,14 +63,14 @@ exit:
 ;   A[-(1 << 62)] = 1;
 ; }
 ;
-; FIXME: There is a dependency between the two stores in all directions.
+; There is a dependency between the two stores in all directions.
 ;
 define void @weak_zero_dst_siv_large_btc(ptr %A) {
 ; CHECK-ALL-LABEL: 'weak_zero_dst_siv_large_btc'
 ; 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 - output [S]!
 ;
@@ -78,7 +78,7 @@ define void @weak_zero_dst_siv_large_btc(ptr %A) {
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 0, ptr %gep.0, align 1
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - output [*]!
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:  Src: store i8 0, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
-; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - none!
+; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - output [*|<]!
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:  Src: store i8 1, ptr %gep.1, align 1 --> Dst: store i8 1, ptr %gep.1, align 1
 ; CHECK-WEAK-ZERO-SRC-SIV-NEXT:    da analyze - output [S]!
 ;

``````````

</details>


https://github.com/llvm/llvm-project/pull/188212