Skip to content

[DA] Check nsw when extracting a constant operand of SCEVMul #164408

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
Open
Parent: [DA] Add tests where dependencies are missed due to overflow (NFC)
wants to merge 1 commit into
base: users/kasuga-fj/da-add-tests-for-overflow
Choose a base branch
from
Open

[DA] Check nsw when extracting a constant operand of SCEVMul #164408

wants to merge 1 commit into from
+14 −16

Conversation

@kasuga-fj
Copy link
Contributor

@kasuga-fj kasuga-fj commented Oct 21, 2025 •
edited
Loading

Given a SCEVMulExpr such as 5 * %m, gcdMIVtest in DA assumes the value as a multiple of 5 in a mathematical sense. However, this is not necessarily true if 5 * %m overflows, especially because an odd number has an inverse modulo 2^64. Such incorrect assumptions can lead to invalid analysis results.
This patch stops unconditionally extracting a constant operand from SCEVMulExpr. Instead, it only allows this when the SCEVMulExpr has the nsw flag.

This was referenced Oct 21, 2025
Merged
Open
@kasuga-fj Graphite App
Copy link
Contributor Author

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.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kasuga-fj
Copy link
Contributor Author

As for the changes in the regression tests, I think the original results are incorrect. They seem to have special corner cases like the one added in #164246.

@kasuga-fj kasuga-fj marked this pull request as ready for review October 21, 2025 14:08
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Oct 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

Changes

Given a SCEVMulExpr such as 5 * %m, gcdMIVtest in DA assumes the value as a multiple of 5 in a mathematical sense. However, this is not necessarily true if 5 * %m overflows, especially because an odd number has an inverse modulo 2^64. Such incorrect assumptions can lead to invalid analysis results.
This patch stops unconditionally extracting a constant operand from SCEVMulExpr. Instead, it only allows this when the SCEVMulExpr has the nsw flag.

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

4 Files Affected:

  • (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (+3-2)
  • (modified) llvm/test/Analysis/DependenceAnalysis/GCD.ll (+3-3)
  • (modified) llvm/test/Analysis/DependenceAnalysis/SymbolicSIV.ll (+2-2)
  • (modified) llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll (+6-9)
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 853bd66c8a7f8..85500d3ffe4ad 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -2828,8 +2828,9 @@ static std::optional<APInt> getConstantPart(const SCEV *Expr) {
   if (const auto *Constant = dyn_cast<SCEVConstant>(Expr))
     return Constant->getAPInt();
   if (const auto *Product = dyn_cast<SCEVMulExpr>(Expr))
-    if (const auto *Constant = dyn_cast<SCEVConstant>(Product->getOperand(0)))
-      return Constant->getAPInt();
+    if (Product->hasNoSignedWrap())
+      if (const auto *Constant = dyn_cast<SCEVConstant>(Product->getOperand(0)))
+        return Constant->getAPInt();
   return std::nullopt;
 }
 
diff --git a/llvm/test/Analysis/DependenceAnalysis/GCD.ll b/llvm/test/Analysis/DependenceAnalysis/GCD.ll
index 03343e7a98211..cb14d189afe4c 100644
--- a/llvm/test/Analysis/DependenceAnalysis/GCD.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/GCD.ll
@@ -254,7 +254,7 @@ define void @gcd4(ptr %A, ptr %B, i64 %M, i64 %N) nounwind uwtable ssp {
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
 ; CHECK-NEXT:    da analyze - output [* *]!
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx16, align 4
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - flow [* *|<]!
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.11, align 4
 ; CHECK-NEXT:    da analyze - confused!
 ; CHECK-NEXT:  Src: %0 = load i32, ptr %arrayidx16, align 4 --> Dst: %0 = load i32, ptr %arrayidx16, align 4
@@ -322,7 +322,7 @@ define void @gcd5(ptr %A, ptr %B, i64 %M, i64 %N) nounwind uwtable ssp {
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %conv, ptr %arrayidx, align 4
 ; CHECK-NEXT:    da analyze - output [* *]!
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: %0 = load i32, ptr %arrayidx16, align 4
-; CHECK-NEXT:    da analyze - flow [<> *]!
+; CHECK-NEXT:    da analyze - flow [* *|<]!
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %0, ptr %B.addr.11, align 4
 ; CHECK-NEXT:    da analyze - confused!
 ; CHECK-NEXT:  Src: %0 = load i32, ptr %arrayidx16, align 4 --> Dst: %0 = load i32, ptr %arrayidx16, align 4
@@ -390,7 +390,7 @@ define void @gcd6(i64 %n, ptr %A, ptr %B) nounwind uwtable ssp {
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx5, align 4 --> Dst: store i32 %conv, ptr %arrayidx5, align 4
 ; CHECK-NEXT:    da analyze - output [* *]!
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx5, align 4 --> Dst: %2 = load i32, ptr %arrayidx9, align 4
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - flow [* *|<]!
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx5, align 4 --> Dst: store i32 %2, ptr %B.addr.12, align 4
 ; CHECK-NEXT:    da analyze - confused!
 ; CHECK-NEXT:  Src: %2 = load i32, ptr %arrayidx9, align 4 --> Dst: %2 = load i32, ptr %arrayidx9, align 4
diff --git a/llvm/test/Analysis/DependenceAnalysis/SymbolicSIV.ll b/llvm/test/Analysis/DependenceAnalysis/SymbolicSIV.ll
index cdfaec76fa892..73a415baef4c4 100644
--- a/llvm/test/Analysis/DependenceAnalysis/SymbolicSIV.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/SymbolicSIV.ll
@@ -384,7 +384,7 @@ define void @symbolicsiv6(ptr %A, ptr %B, i64 %n, i64 %N, i64 %M) nounwind uwtab
 ; 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 %arrayidx7, align 4
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - flow [*|<]!
 ; 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 %arrayidx7, align 4 --> Dst: %0 = load i32, ptr %arrayidx7, align 4
@@ -440,7 +440,7 @@ define void @symbolicsiv7(ptr %A, ptr %B, i64 %n, i64 %N, i64 %M) nounwind uwtab
 ; 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: %1 = load i32, ptr %arrayidx6, align 4
-; CHECK-NEXT:    da analyze - flow [<>]!
+; CHECK-NEXT:    da analyze - flow [*|<]!
 ; CHECK-NEXT:  Src: store i32 %conv, ptr %arrayidx, align 4 --> Dst: store i32 %1, ptr %B.addr.02, align 4
 ; CHECK-NEXT:    da analyze - confused!
 ; CHECK-NEXT:  Src: %1 = load i32, ptr %arrayidx6, align 4 --> Dst: %1 = load i32, ptr %arrayidx6, align 4
diff --git a/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
index 724b347b56f3a..b16133af71a1f 100644
--- a/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/gcd-miv-overflow.ll
@@ -12,23 +12,20 @@
 ;   offset1 += 3;
 ; }
 ;
-; FIXME: DependenceAnalysis currently detects no dependency between the two
-; stores, but it does exist. E.g., consider `m` is 12297829382473034411, which
-; is a modular multiplicative inverse of 3 under modulo 2^64. Then `offset0` is
-; effectively `i + 4`, so accesses will be as follows:
+; Dependency exists between the two stores. E.g., consider `m` is
+; 12297829382473034411, which is a modular multiplicative inverse of 3 under
+; modulo 2^64. Then `offset0` is effectively `i + 4`, so accesses will be as
+; follows:
 ;
 ;   - A[offset0] : A[4], A[5], A[6], ...
 ;   - A[offset1] : A[0], A[3], A[6], ...
 ;
-; The root cause is that DA assumes `3*m` begin a multiple of 3 in mathematical
-; sense, which isn't necessarily true due to overflow.
-;
 define void @gcdmiv_coef_ovfl(ptr %A, i64 %m) {
 ; CHECK-LABEL: 'gcdmiv_coef_ovfl'
 ; CHECK-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
 ; CHECK-NEXT:    da analyze - none!
 ; CHECK-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
-; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:    da analyze - output [*|<]!
 ; CHECK-NEXT:  Src: store i8 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
 ; CHECK-NEXT:    da analyze - none!
 ;
@@ -36,7 +33,7 @@ define void @gcdmiv_coef_ovfl(ptr %A, i64 %m) {
 ; CHECK-GCD-MIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 1, ptr %gep.0, align 1
 ; CHECK-GCD-MIV-NEXT:    da analyze - consistent output [*]!
 ; CHECK-GCD-MIV-NEXT:  Src: store i8 1, ptr %gep.0, align 1 --> Dst: store i8 2, 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 2, ptr %gep.1, align 1 --> Dst: store i8 2, ptr %gep.1, align 1
 ; CHECK-GCD-MIV-NEXT:    da analyze - consistent output [*]!
 ;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Meinersbur Meinersbur Awaiting requested review from Meinersbur

@sjoerdmeijer sjoerdmeijer Awaiting requested review from sjoerdmeijer

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.

2 participants

@kasuga-fj @llvmbot