[DA] Let getConstantPart return optional APInt (NFC)

[artagnon]

To use the result of an SCEVConstant, we need to extract the APInt, which callers anyway do.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

---

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

1 Files Affected:

• (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (+24-31)

diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index c1b1d002c9979..1cc756d367aea 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -2375,17 +2375,15 @@ bool DependenceInfo::testMIV(const SCEV *Src, const SCEV *Dst,
 
 // Given a product, e.g., 10*X*Y, returns the first constant operand,
 // in this case 10. If there is no constant part, returns NULL.
-static
-const SCEVConstant *getConstantPart(const SCEV *Expr) {
+static std::optional<APInt> getConstantPart(const SCEV *Expr) {
   if (const auto *Constant = dyn_cast<SCEVConstant>(Expr))
-    return Constant;
-  else if (const auto *Product = dyn_cast<SCEVMulExpr>(Expr))
+    return Constant->getAPInt();
+  if (const auto *Product = dyn_cast<SCEVMulExpr>(Expr))
     if (const auto *Constant = dyn_cast<SCEVConstant>(Product->getOperand(0)))
-      return Constant;
-  return nullptr;
+      return Constant->getAPInt();
+  return std::nullopt;
 }
 
-
 //===----------------------------------------------------------------------===//
 // gcdMIVtest -
 // Tests an MIV subscript pair for dependence.
@@ -2421,11 +2419,10 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
     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.
-    const auto *Constant = getConstantPart(Coeff);
-    if (!Constant)
+    std::optional<APInt> ConstCoeff = getConstantPart(Coeff);
+    if (!ConstCoeff)
       return false;
-    APInt ConstCoeff = Constant->getAPInt();
-    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff.abs());
+    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
     Coefficients = AddRec->getStart();
   }
   const SCEV *SrcConst = Coefficients;
@@ -2440,11 +2437,10 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
     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.
-    const auto *Constant = getConstantPart(Coeff);
-    if (!Constant)
+    std::optional<APInt> ConstCoeff = getConstantPart(Coeff);
+    if (!ConstCoeff)
       return false;
-    APInt ConstCoeff = Constant->getAPInt();
-    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff.abs());
+    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
     Coefficients = AddRec->getStart();
   }
   const SCEV *DstConst = Coefficients;
@@ -2463,12 +2459,10 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
       else if (const SCEVMulExpr *Product = dyn_cast<SCEVMulExpr>(Operand)) {
         // Search for constant operand to participate in GCD;
         // If none found; return false.
-        const SCEVConstant *ConstOp = getConstantPart(Product);
+        std::optional<APInt> ConstOp = getConstantPart(Product);
         if (!ConstOp)
           return false;
-        APInt ConstOpValue = ConstOp->getAPInt();
-        ExtraGCD = APIntOps::GreatestCommonDivisor(ExtraGCD,
-                                                   ConstOpValue.abs());
+        ExtraGCD = APIntOps::GreatestCommonDivisor(ExtraGCD, ConstOp->abs());
       }
       else
         return false;
@@ -2520,11 +2514,11 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
       else {
         // If the coefficient is the product of a constant and other stuff,
         // we can use the constant in the GCD computation.
-        Constant = getConstantPart(Coeff);
-        if (!Constant)
+        std::optional<APInt> ConstCoeff = getConstantPart(Coeff);
+        if (!ConstCoeff)
           return false;
-        APInt ConstCoeff = Constant->getAPInt();
-        RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff.abs());
+        RunningGCD =
+            APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
       }
       Inner = AddRec->getStart();
     }
@@ -2537,24 +2531,23 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
       else {
         // If the coefficient is the product of a constant and other stuff,
         // we can use the constant in the GCD computation.
-        Constant = getConstantPart(Coeff);
-        if (!Constant)
+        std::optional<APInt> ConstCoeff = getConstantPart(Coeff);
+        if (!ConstCoeff)
           return false;
-        APInt ConstCoeff = Constant->getAPInt();
-        RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff.abs());
+        RunningGCD =
+            APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
       }
       Inner = AddRec->getStart();
     }
     Delta = SE->getMinusSCEV(SrcCoeff, DstCoeff);
     // If the coefficient is the product of a constant and other stuff,
     // we can use the constant in the GCD computation.
-    Constant = getConstantPart(Delta);
-    if (!Constant)
+    std::optional<APInt> ConstCoeff = getConstantPart(Delta);
+    if (!ConstCoeff)
       // The difference of the two coefficients might not be a product
       // or constant, in which case we give up on this direction.
       continue;
-    APInt ConstCoeff = Constant->getAPInt();
-    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff.abs());
+    RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
     LLVM_DEBUG(dbgs() << "\tRunningGCD = " << RunningGCD << "\n");
     if (RunningGCD != 0) {
       Remainder = ConstDelta.srem(RunningGCD);

[kasuga-fj]

The goal of this change is to extract the common logic, right? If so, should we return the absolute value of the APInt instead?

[artagnon]

The goal of this change is to extract the common logic, right? If so, should we return the absolute value of the APInt instead?

I would prefer not to return the absolute value, as it would violate the meaning of the function name. There is already precedent for returning an APInt from a SCEVConstant like in SCEVPatternMatch (although it would be best not to use SCEVPatternMatch in DA).

[kasuga-fj]

I meant the motivation for this PR, which I personally think should be mentioned in the PR description.

Anyway the code itself looks good to me.