Skip to content

Conversation

@kasuga-fj
Copy link
Contributor

@kasuga-fj kasuga-fj commented Dec 11, 2025

If two subscripts has same cast operation (sext or zext), removeMatchingExtensions strips off them. Due to the following reasons, remaining this function is not useful and risky:

  • If the operand of the cast operation has proper nowrap properties, SCEV usually strips off the cast operation automatically. It's not very meaningful to do it on DA side.
  • If the operand doesn't have nowrap properties, stripping off the cast operation may lead to incorrect result. Especially, if it can be negative, removing zext would change the interpretation of the value in a signed sense.
  • This function is part of the root cause of [DA] SIV test expected at least one AddRec #148435.

Although I've not found any cases where incorrect results are produced by this function, but I think it doesn't make sense to keep it. As far as I have checked, there are no regressions after removing this function.

Resolve #169809

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

llvmbot commented Dec 11, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

Changes

If two subscripts has same cast operation (sext or zext), removeMatchingExtensions strips off them. Due to the following reasons, remaining this function is not useful and potentially leads to unexpected behavior:

  • If the operand of the cast operation has proper nowrap properties, SCEV usually strips off the cast operation automatically. It's not very meaningful to do it on DA side.
  • If the operand doesn't have nowrap properties, stripping off the cast operation may lead to incorrect result. Especially, if it can be negative, removing zext would change the interpretation of the value in a signed sense.
  • This function is part of the root cause of #148435.

Although I've not found any cases where incorrect results are produced by this function, but I think it doesn't make sense to keep it. As far as I have checked, there are no regressions after removing this function.

Resolve #169809


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

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/DependenceAnalysis.h (-6)
  • (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (-21)
diff --git a/llvm/include/llvm/Analysis/DependenceAnalysis.h b/llvm/include/llvm/Analysis/DependenceAnalysis.h
index 6dec24fc9f104..d8dfeb7f055ad 100644
--- a/llvm/include/llvm/Analysis/DependenceAnalysis.h
+++ b/llvm/include/llvm/Analysis/DependenceAnalysis.h
@@ -478,12 +478,6 @@ class DependenceInfo {
   /// array subscripts are signed.
   void unifySubscriptType(ArrayRef<Subscript *> Pairs);
 
-  /// removeMatchingExtensions - Examines a subscript pair.
-  /// If the source and destination are identically sign (or zero)
-  /// extended, it strips off the extension in an effort to
-  /// simplify the actual analysis.
-  void removeMatchingExtensions(Subscript *Pair);
-
   /// collectCommonLoops - Finds the set of loops from the LoopNest that
   /// have a level <= CommonLevels and are referred to by the SCEV Expression.
   void collectCommonLoops(const SCEV *Expression, const Loop *LoopNest,
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 9b9c80a9b3266..e3a0da964a3b3 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -1030,26 +1030,6 @@ void DependenceInfo::unifySubscriptType(ArrayRef<Subscript *> Pairs) {
   }
 }
 
-// removeMatchingExtensions - Examines a subscript pair.
-// If the source and destination are identically sign (or zero)
-// extended, it strips off the extension in an effect to simplify
-// the actual analysis.
-void DependenceInfo::removeMatchingExtensions(Subscript *Pair) {
-  const SCEV *Src = Pair->Src;
-  const SCEV *Dst = Pair->Dst;
-  if ((isa<SCEVZeroExtendExpr>(Src) && isa<SCEVZeroExtendExpr>(Dst)) ||
-      (isa<SCEVSignExtendExpr>(Src) && isa<SCEVSignExtendExpr>(Dst))) {
-    const SCEVIntegralCastExpr *SrcCast = cast<SCEVIntegralCastExpr>(Src);
-    const SCEVIntegralCastExpr *DstCast = cast<SCEVIntegralCastExpr>(Dst);
-    const SCEV *SrcCastOp = SrcCast->getOperand();
-    const SCEV *DstCastOp = DstCast->getOperand();
-    if (SrcCastOp->getType() == DstCastOp->getType()) {
-      Pair->Src = SrcCastOp;
-      Pair->Dst = DstCastOp;
-    }
-  }
-}
-
 // Examine the scev and return true iff it's affine.
 // Collect any loops mentioned in the set of "Loops".
 bool DependenceInfo::checkSubscript(const SCEV *Expr, const Loop *LoopNest,
@@ -3556,7 +3536,6 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
     Pair[P].Loops.resize(MaxLevels + 1);
     Pair[P].GroupLoops.resize(MaxLevels + 1);
     Pair[P].Group.resize(Pairs);
-    removeMatchingExtensions(&Pair[P]);
     Pair[P].Classification =
         classifyPair(Pair[P].Src, LI->getLoopFor(Src->getParent()), Pair[P].Dst,
                      LI->getLoopFor(Dst->getParent()), Pair[P].Loops);

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DA] Fix function removeMatchExtensions

2 participants