[DA] factor out repetitive code in GCD test (NFCI)

[amehsan]

The logic for recursively investigating the source and destination AddRecs in GCD test is the same and can be factored out.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Ehsan Amiri (amehsan)

Changes

The logic for recursively investigating the source and destination AddRecs in GCD test is the same and can be factored out.

---

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

1 Files Affected:

• (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (+29-36)

diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 76f514e4c9327..e6d717f0a5b74 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -2212,6 +2212,27 @@ bool DependenceInfo::accumulateCoefficientsGCD(const SCEV *Expr,
   return accumulateCoefficientsGCD(Start, CurLoop, CurLoopCoeff, RunningGCD);
 }
 
+// Compute running GCD and record source constant.
+// Because we're looking for the constant at the end of the chain,
+// we can't quit the loop just because the GCD == 1.
+const SCEV* analyzeCoefficientsForGCD(const SCEV *Coefficients, 
+                                      APInt &RunningGCD,
+                                      ScalarEvolution *SE)
+{
+  while (const SCEVAddRecExpr *AddRec =
+             dyn_cast<SCEVAddRecExpr>(Coefficients)) {
+    const SCEV *Coeff = AddRec->getStepRecurrence(*SE);
+    // If the coefficient is the product of a constant and other stuff,
+    // we can use the constant in the GCD computation.
+    std::optional<APInt> ConstCoeff = getConstantCoefficient(Coeff);
+    if (!ConstCoeff)
+      return nullptr;
+    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
+    Coefficients = AddRec->getStart();
+  }
+  return Coefficients;
+}
+
 //===----------------------------------------------------------------------===//
 // gcdMIVtest -
 // Tests an MIV subscript pair for dependence.
@@ -2239,41 +2260,13 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
   unsigned BitWidth = SE->getTypeSizeInBits(Src->getType());
   APInt RunningGCD = APInt::getZero(BitWidth);
 
-  // Examine Src coefficients.
-  // Compute running GCD and record source constant.
-  // Because we're looking for the constant at the end of the chain,
-  // we can't quit the loop just because the GCD == 1.
-  const SCEV *Coefficients = Src;
-  while (const SCEVAddRecExpr *AddRec =
-             dyn_cast<SCEVAddRecExpr>(Coefficients)) {
-    const SCEV *Coeff = AddRec->getStepRecurrence(*SE);
-    // If the coefficient is the product of a constant and other stuff,
-    // we can use the constant in the GCD computation.
-    std::optional<APInt> ConstCoeff = getConstantCoefficient(Coeff);
-    if (!ConstCoeff)
-      return false;
-    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
-    Coefficients = AddRec->getStart();
-  }
-  const SCEV *SrcConst = Coefficients;
-
-  // Examine Dst coefficients.
-  // Compute running GCD and record destination constant.
-  // Because we're looking for the constant at the end of the chain,
-  // we can't quit the loop just because the GCD == 1.
-  Coefficients = Dst;
-  while (const SCEVAddRecExpr *AddRec =
-             dyn_cast<SCEVAddRecExpr>(Coefficients)) {
-    const SCEV *Coeff = AddRec->getStepRecurrence(*SE);
-    // If the coefficient is the product of a constant and other stuff,
-    // we can use the constant in the GCD computation.
-    std::optional<APInt> ConstCoeff = getConstantCoefficient(Coeff);
-    if (!ConstCoeff)
-      return false;
-    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
-    Coefficients = AddRec->getStart();
-  }
-  const SCEV *DstConst = Coefficients;
+  // Examine Src and dst coefficients.
+  const SCEV *SrcConst = analyzeCoefficientsForGCD(Src, RunningGCD, SE);
+  if (!SrcConst)
+    return false;
+  const SCEV *DstConst = analyzeCoefficientsForGCD(Dst, RunningGCD, SE);
+  if (!DstConst)
+    return false;
 
   APInt ExtraGCD = APInt::getZero(BitWidth);
   const SCEV *Delta = minusSCEVNoSignedOverflow(DstConst, SrcConst, *SE);
@@ -2309,7 +2302,7 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
   LLVM_DEBUG(dbgs() << "    ExtraGCD = " << ExtraGCD << '\n');
 
   bool Improved = false;
-  Coefficients = Src;
+  const SCEV *Coefficients = Src;
   while (const SCEVAddRecExpr *AddRec =
              dyn_cast<SCEVAddRecExpr>(Coefficients)) {
     Coefficients = AddRec->getStart();

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[cabbaken]

The comment ("source" should be deleted now) needs to be refined.

[amehsan]

Comment Revised

[kasuga-fj]

Suggested change

	// Compute running GCD and record source constant.
	// Because we're looking for the constant at the end of the chain,
	// we can't quit the loop just because the GCD == 1.
	/// Compute \p RunningGCD and record source constant.
	/// Because we're looking for the constant at the end of the chain,
	/// we can't quit the loop just because \p RunningGCD == 1.

Use Doxygen‑style comments.

[amehsan]

Done

[cabbaken]

LGTM

[kasuga-fj]

I realized that there is an almost identical function DependenceInfo::accumulateCoefficientsGCD right above. Consider extending it rather than creating a new one.

[amehsan]

I realized that there is an almost identical function DependenceInfo::accumulateCoefficientsGCD right above. Consider extending it rather than creating a new one.

I will check that in the next step to see if more code can be commoned.

[kasuga-fj]

I'll take care of it myself... I took a closer look and it was a bit off, so never mind.