Skip to content

Commit bce0f9d

Browse files
authored
[DA] Extract duplicated logic from gcdMIVtest (NFCI) (#152688)
This patch refactors `gcdMIVtest` by consolidating duplicated logic into a single function. The main goal of this change is to improve code maintainability rather than readability, especially since we may need to revise this logic for correctness (as noted in the added TODO comments). I hope this patch is NFC, but I've also added several new assertions, which may cause some previously passing cases to fail.
1 parent 09505b1 commit bce0f9d

File tree

2 files changed

+61
-34
lines changed

2 files changed

+61
-34
lines changed

llvm/include/llvm/Analysis/DependenceAnalysis.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,25 @@ class DependenceInfo {
765765
CoefficientInfo *collectCoeffInfo(const SCEV *Subscript, bool SrcFlag,
766766
const SCEV *&Constant) const;
767767

768+
/// Given \p Expr of the form
769+
///
770+
/// c_0*X_0*i_0 + c_1*X_1*i_1 + ...c_n*X_n*i_n + C
771+
///
772+
/// compute
773+
///
774+
/// RunningGCD = gcd(RunningGCD, c_0, c_1, ..., c_n)
775+
///
776+
/// where c_0, c_1, ..., and c_n are the constant values. The result is stored
777+
/// in \p RunningGCD. Also, the initial value of \p RunningGCD affects the
778+
/// result. If we find a term like (c_k * X_k * i_k), where i_k is the
779+
/// induction variable of \p CurLoop, c_k is stored in \p CurLoopCoeff and not
780+
/// included in the GCD computation. Returns false if we fail to find a
781+
/// constant coefficient for some loop, e.g., when a term like (X+Y)*i is
782+
/// present. Otherwise returns true.
783+
bool accumulateCoefficientsGCD(const SCEV *Expr, const Loop *CurLoop,
784+
const SCEV *&CurLoopCoeff,
785+
APInt &RunningGCD) const;
786+
768787
/// getPositivePart - X^+ = max(X, 0).
769788
const SCEV *getPositivePart(const SCEV *X) const;
770789

llvm/lib/Analysis/DependenceAnalysis.cpp

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2345,6 +2345,43 @@ static std::optional<APInt> getConstantPart(const SCEV *Expr) {
23452345
return std::nullopt;
23462346
}
23472347

2348+
bool DependenceInfo::accumulateCoefficientsGCD(const SCEV *Expr,
2349+
const Loop *CurLoop,
2350+
const SCEV *&CurLoopCoeff,
2351+
APInt &RunningGCD) const {
2352+
// If RunningGCD is already 1, exit early.
2353+
// TODO: It might be better to continue the recursion to find CurLoopCoeff.
2354+
if (RunningGCD == 1)
2355+
return true;
2356+
2357+
const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(Expr);
2358+
if (!AddRec) {
2359+
assert(isLoopInvariant(Expr, CurLoop) &&
2360+
"Expected loop invariant expression");
2361+
return true;
2362+
}
2363+
2364+
assert(AddRec->isAffine() && "Unexpected Expr");
2365+
const SCEV *Start = AddRec->getStart();
2366+
const SCEV *Step = AddRec->getStepRecurrence(*SE);
2367+
if (AddRec->getLoop() == CurLoop) {
2368+
CurLoopCoeff = Step;
2369+
} else {
2370+
std::optional<APInt> ConstCoeff = getConstantPart(Step);
2371+
2372+
// If the coefficient is the product of a constant and other stuff, we can
2373+
// use the constant in the GCD computation.
2374+
if (!ConstCoeff)
2375+
return false;
2376+
2377+
// TODO: What happens if ConstCoeff is the "most negative" signed number
2378+
// (e.g. -128 for 8 bit wide APInt)?
2379+
RunningGCD = APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
2380+
}
2381+
2382+
return accumulateCoefficientsGCD(Start, CurLoop, CurLoopCoeff, RunningGCD);
2383+
}
2384+
23482385
//===----------------------------------------------------------------------===//
23492386
// gcdMIVtest -
23502387
// Tests an MIV subscript pair for dependence.
@@ -2464,40 +2501,11 @@ bool DependenceInfo::gcdMIVtest(const SCEV *Src, const SCEV *Dst,
24642501
RunningGCD = ExtraGCD;
24652502
const SCEV *SrcCoeff = AddRec->getStepRecurrence(*SE);
24662503
const SCEV *DstCoeff = SE->getMinusSCEV(SrcCoeff, SrcCoeff);
2467-
const SCEV *Inner = Src;
2468-
while (RunningGCD != 1 && isa<SCEVAddRecExpr>(Inner)) {
2469-
AddRec = cast<SCEVAddRecExpr>(Inner);
2470-
const SCEV *Coeff = AddRec->getStepRecurrence(*SE);
2471-
if (CurLoop == AddRec->getLoop())
2472-
; // SrcCoeff == Coeff
2473-
else {
2474-
// If the coefficient is the product of a constant and other stuff,
2475-
// we can use the constant in the GCD computation.
2476-
std::optional<APInt> ConstCoeff = getConstantPart(Coeff);
2477-
if (!ConstCoeff)
2478-
return false;
2479-
RunningGCD =
2480-
APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
2481-
}
2482-
Inner = AddRec->getStart();
2483-
}
2484-
Inner = Dst;
2485-
while (RunningGCD != 1 && isa<SCEVAddRecExpr>(Inner)) {
2486-
AddRec = cast<SCEVAddRecExpr>(Inner);
2487-
const SCEV *Coeff = AddRec->getStepRecurrence(*SE);
2488-
if (CurLoop == AddRec->getLoop())
2489-
DstCoeff = Coeff;
2490-
else {
2491-
// If the coefficient is the product of a constant and other stuff,
2492-
// we can use the constant in the GCD computation.
2493-
std::optional<APInt> ConstCoeff = getConstantPart(Coeff);
2494-
if (!ConstCoeff)
2495-
return false;
2496-
RunningGCD =
2497-
APIntOps::GreatestCommonDivisor(RunningGCD, ConstCoeff->abs());
2498-
}
2499-
Inner = AddRec->getStart();
2500-
}
2504+
2505+
if (!accumulateCoefficientsGCD(Src, CurLoop, SrcCoeff, RunningGCD) ||
2506+
!accumulateCoefficientsGCD(Dst, CurLoop, DstCoeff, RunningGCD))
2507+
return false;
2508+
25012509
Delta = SE->getMinusSCEV(SrcCoeff, DstCoeff);
25022510
// If the coefficient is the product of a constant and other stuff,
25032511
// we can use the constant in the GCD computation.

0 commit comments

Comments
 (0)