Skip to content

Revert "[DA] Check for overflow in strong SIV test" #165905

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 31, 2025
Merged

Revert "[DA] Check for overflow in strong SIV test" #165905

merged 1 commit into from
Oct 31, 2025

Conversation

@amehsan
Copy link
Contributor

@amehsan amehsan commented Oct 31, 2025 •
edited by CongzheUalberta
Loading

Reverts #164704 that broke several built bots: #164704 (comment)

@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Oct 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ehsan Amiri (amehsan)

Changes

Reverts llvm/llvm-project#164704

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

3 Files Affected:

  • (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (+2-17)
  • (modified) llvm/test/Analysis/DependenceAnalysis/SimpleSIVNoValidityCheck.ll (+1-1)
  • (modified) llvm/test/Analysis/DependenceAnalysis/StrongSIV.ll (+14-72)
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index e45d1f79b3165..11d829492a10e 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -1587,15 +1587,6 @@ static const SCEV *minusSCEVNoSignedOverflow(const SCEV *A, const SCEV *B,
   return nullptr;
 }
 
-/// Returns \p A * \p B if it guaranteed not to signed wrap. Otherwise returns
-/// nullptr. \p A and \p B must have the same integer type.
-static const SCEV *mulSCEVNoSignedOverflow(const SCEV *A, const SCEV *B,
-                                           ScalarEvolution &SE) {
-  if (SE.willNotOverflow(Instruction::Mul, /*Signed=*/true, A, B))
-    return SE.getMulExpr(A, B);
-  return nullptr;
-}
-
 /// Returns the absolute value of \p A. In the context of dependence analysis,
 /// we need an absolute value in a mathematical sense. If \p A is the signed
 /// minimum value, we cannot represent it unless extending the original type.
@@ -1695,11 +1686,7 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
   assert(0 < Level && Level <= CommonLevels && "level out of range");
   Level--;
 
-  const SCEV *Delta = minusSCEVNoSignedOverflow(SrcConst, DstConst, *SE);
-  if (!Delta) {
-    Result.Consistent = false;
-    return false;
-  }
+  const SCEV *Delta = SE->getMinusSCEV(SrcConst, DstConst);
   LLVM_DEBUG(dbgs() << "\t    Delta = " << *Delta);
   LLVM_DEBUG(dbgs() << ", " << *Delta->getType() << "\n");
 
@@ -1715,9 +1702,7 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
     const SCEV *AbsCoeff = absSCEVNoSignedOverflow(Coeff, *SE);
     if (!AbsDelta || !AbsCoeff)
       return false;
-    const SCEV *Product = mulSCEVNoSignedOverflow(UpperBound, AbsCoeff, *SE);
-    if (!Product)
-      return false;
+    const SCEV *Product = SE->getMulExpr(UpperBound, AbsCoeff);
     return isKnownPredicate(CmpInst::ICMP_SGT, AbsDelta, Product);
   }();
   if (IsDeltaLarge) {
diff --git a/llvm/test/Analysis/DependenceAnalysis/SimpleSIVNoValidityCheck.ll b/llvm/test/Analysis/DependenceAnalysis/SimpleSIVNoValidityCheck.ll
index 181a4494b036e..4346507ba8f90 100644
--- a/llvm/test/Analysis/DependenceAnalysis/SimpleSIVNoValidityCheck.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/SimpleSIVNoValidityCheck.ll
@@ -210,7 +210,7 @@ define void @t3(i64 %n, i64 %m, i64 %lb, ptr %a) {
 ; CHECK-NEXT:  Src: %2 = load i32, ptr %arrayidx6, align 4 --> Dst: %2 = load i32, ptr %arrayidx6, align 4
 ; CHECK-NEXT:    da analyze - none!
 ; CHECK-NEXT:  Src: %2 = load i32, ptr %arrayidx6, align 4 --> Dst: store i32 %2, ptr %arrayidx8, align 4
-; CHECK-NEXT:    da analyze - anti [1 *]!
+; CHECK-NEXT:    da analyze - consistent anti [1 -2]!
 ; CHECK-NEXT:  Src: store i32 %2, ptr %arrayidx8, align 4 --> Dst: store i32 %2, ptr %arrayidx8, align 4
 ; CHECK-NEXT:    da analyze - none!
 ;
diff --git a/llvm/test/Analysis/DependenceAnalysis/StrongSIV.ll b/llvm/test/Analysis/DependenceAnalysis/StrongSIV.ll
index 160196284f415..44bd9b7727910 100644
--- a/llvm/test/Analysis/DependenceAnalysis/StrongSIV.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/StrongSIV.ll
@@ -1,8 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 \
-; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-ALL
-; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa -da-enable-dependence-test=strong-siv 2>&1 \
-; RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-STRONG-SIV
+; RUN: | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.6.0"
@@ -425,33 +423,19 @@ for.end:                                          ; preds = %for.body
 ;;    *B++ = A[i + 2*n];
 
 define void @strong9(ptr %A, ptr %B, i64 %n) nounwind uwtable ssp {
-; CHECK-ALL-LABEL: 'strong9'
-; CHECK-ALL-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
-; CHECK-ALL-NEXT:    da analyze - none!
-; CHECK-ALL-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
-; CHECK-ALL-NEXT:    da analyze - none!
-; CHECK-ALL-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
-; CHECK-ALL-NEXT:    da analyze - confused!
-; CHECK-ALL-NEXT:  Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
-; CHECK-ALL-NEXT:    da analyze - none!
-; CHECK-ALL-NEXT:  Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
-; CHECK-ALL-NEXT:    da analyze - confused!
-; CHECK-ALL-NEXT:  Src: store i32 %0, ptr %B.addr.02, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
-; CHECK-ALL-NEXT:    da analyze - none!
-;
-; CHECK-STRONG-SIV-LABEL: 'strong9'
-; CHECK-STRONG-SIV-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
-; CHECK-STRONG-SIV-NEXT:    da analyze - none!
-; CHECK-STRONG-SIV-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
-; CHECK-STRONG-SIV-NEXT:    da analyze - flow [*|<]!
-; CHECK-STRONG-SIV-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
-; CHECK-STRONG-SIV-NEXT:    da analyze - confused!
-; CHECK-STRONG-SIV-NEXT:  Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
-; CHECK-STRONG-SIV-NEXT:    da analyze - none!
-; CHECK-STRONG-SIV-NEXT:  Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
-; CHECK-STRONG-SIV-NEXT:    da analyze - confused!
-; CHECK-STRONG-SIV-NEXT:  Src: store i32 %0, ptr %B.addr.02, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
-; CHECK-STRONG-SIV-NEXT:    da analyze - none!
+; CHECK-LABEL: 'strong9'
+; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
+; CHECK-NEXT:    da analyze - confused!
+; CHECK-NEXT:  Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: %0 = load i32, ptr %arrayidx2, align 4
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: %0 = load i32, ptr %arrayidx2, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
+; CHECK-NEXT:    da analyze - confused!
+; CHECK-NEXT:  Src: store i32 %0, ptr %B.addr.02, align 4 --> Dst: store i32 %0, ptr %B.addr.02, align 4
+; CHECK-NEXT:    da analyze - none!
 ;
 entry:
   %cmp1 = icmp eq i64 %n, 0
@@ -528,45 +512,3 @@ for.body:                                         ; preds = %entry, %for.body
 for.end:                                          ; preds = %for.body
   ret void
 }
-
-
-;;  for (long unsigned i = 0; i < 9223372036854775806; i++)
-;;    for (long unsigned j = 0; j < 2147483640; j++)
-;;      if (i < 3000000000)
-;;        A[i] = 0;
-;
-; FIXME: DependenceAnalysis fails to detect the dependency between A[i] and
-; itself, while Strong SIV has been able to prove it.
-define void @strong11(ptr %A) nounwind uwtable ssp {
-; CHECK-ALL-LABEL: 'strong11'
-; CHECK-ALL-NEXT:  Src: store i32 0, ptr %arrayidx, align 4 --> Dst: store i32 0, ptr %arrayidx, align 4
-; CHECK-ALL-NEXT:    da analyze - none!
-;
-; CHECK-STRONG-SIV-LABEL: 'strong11'
-; CHECK-STRONG-SIV-NEXT:  Src: store i32 0, ptr %arrayidx, align 4 --> Dst: store i32 0, ptr %arrayidx, align 4
-; CHECK-STRONG-SIV-NEXT:    da analyze - consistent output [0 S]!
-;
-entry:
-  br label %for.cond1.preheader
-
-for.cond1.preheader:                              ; preds = %entry, %for.cond.cleanup3
-  %i.017 = phi i64 [ 0, %entry ], [ %inc8, %for.cond.cleanup3 ]
-  %cmp5 = icmp samesign ult i64 %i.017, 3000000000
-  %arrayidx = getelementptr inbounds nuw i32, ptr %A, i64 %i.017
-  br i1 %cmp5, label %for.body4.us, label %for.cond.cleanup3
-
-for.body4.us:                                     ; preds = %for.cond1.preheader, %for.body4.us
-  %j.016.us = phi i64 [ %inc.us, %for.body4.us ], [ 0, %for.cond1.preheader ]
-  store i32 0, ptr %arrayidx, align 4
-  %inc.us = add nuw nsw i64 %j.016.us, 1
-  %exitcond.not = icmp eq i64 %inc.us, 2147483640
-  br i1 %exitcond.not, label %for.cond.cleanup3, label %for.body4.us
-
-for.cond.cleanup:                                 ; preds = %for.cond.cleanup3
-  ret void
-
-for.cond.cleanup3:                                ; preds = %for.body4.us, %for.cond1.preheader
-  %inc8 = add nuw nsw i64 %i.017, 1
-  %exitcond19.not = icmp eq i64 %inc8, 9223372036854775806
-  br i1 %exitcond19.not, label %for.cond.cleanup, label %for.cond1.preheader
-}

ilovepi
ilovepi approved these changes Oct 31, 2025
View reviewed changes
Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include the reason for the revert (buildbot failures) in the commit message.

@jansvoboda11
Copy link
Contributor

jansvoboda11 commented Oct 31, 2025

Can we merge this? The libcxx CI has been pending for an hour. (It's been pending on my own PR for over two hours.) I don't think it makes sense to block fixing bots on that check.

@CongzheUalberta CongzheUalberta merged commit 6125f26 into main Oct 31, 2025
10 of 11 checks passed
@CongzheUalberta CongzheUalberta deleted the revert-164704-strong-siv-overflow branch October 31, 2025 19:50
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
@amehsan
Revert "[DA] Check for overflow in strong SIV test" (llvm#165905)
e3be57e 
Reverts llvm#164704 that broke several built bots.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ilovepi ilovepi ilovepi approved these changes

Assignees

No one assigned

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@amehsan @llvmbot @jansvoboda11 @ilovepi @CongzheUalberta