[llvm] [Delinearization] Remove `isKnownLessThan` (PR #171821)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

<details>
<summary>Changes</summary>

Delinearization has `isKnownLessThan(a, b)` function to check if `a < b` is known at compile time. To check the predicate, it calls `isKnownNegative(a - b)`. There are at least two issues with this approach:

- It mixed up a signed interpretation with an unsigned one.
- It doesn't consider overflow.

This function includes some additional logic, which suffers from the same issues. This patch replace the call to `isKnownLessThan(a, b)` with `ScalarEvolution::isKnownPredicate(ICmpInst::ICMP_SLT, a, b)` and removes the former function entirely, since using it is risky and may lead to incorrect results.

Resolve #<!-- -->169812

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


1 Files Affected:

- (modified) llvm/lib/Analysis/Delinearization.cpp (+7-68) 


``````````diff
diff --git a/llvm/lib/Analysis/Delinearization.cpp b/llvm/lib/Analysis/Delinearization.cpp
index 7bf83ccf9c172..c3e3bc33a8b69 100644
--- a/llvm/lib/Analysis/Delinearization.cpp
+++ b/llvm/lib/Analysis/Delinearization.cpp
@@ -686,73 +686,6 @@ static bool isKnownNonNegative(ScalarEvolution *SE, const SCEV *S,
   return SE->isKnownNonNegative(S);
 }
 
-/// Compare to see if S is less than Size, using
-///
-///    isKnownNegative(S - Size)
-///
-/// with some extra checking if S is an AddRec and we can prove less-than using
-/// the loop bounds.
-static bool isKnownLessThan(ScalarEvolution *SE, const SCEV *S,
-                            const SCEV *Size) {
-  // First unify to the same type
-  auto *SType = dyn_cast<IntegerType>(S->getType());
-  auto *SizeType = dyn_cast<IntegerType>(Size->getType());
-  if (!SType || !SizeType)
-    return false;
-  Type *MaxType =
-      (SType->getBitWidth() >= SizeType->getBitWidth()) ? SType : SizeType;
-  S = SE->getTruncateOrZeroExtend(S, MaxType);
-  Size = SE->getTruncateOrZeroExtend(Size, MaxType);
-
-  auto CollectUpperBound = [&](const Loop *L, Type *T) -> const SCEV * {
-    if (SE->hasLoopInvariantBackedgeTakenCount(L)) {
-      const SCEV *UB = SE->getBackedgeTakenCount(L);
-      return SE->getTruncateOrZeroExtend(UB, T);
-    }
-    return nullptr;
-  };
-
-  auto CheckAddRecBECount = [&]() {
-    const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(S);
-    if (!AddRec || !AddRec->isAffine() || !AddRec->hasNoSignedWrap())
-      return false;
-    const SCEV *BECount = CollectUpperBound(AddRec->getLoop(), MaxType);
-    // If the BTC cannot be computed, check the base case for S.
-    if (!BECount || isa<SCEVCouldNotCompute>(BECount))
-      return false;
-    const SCEV *Start = AddRec->getStart();
-    const SCEV *Step = AddRec->getStepRecurrence(*SE);
-    const SCEV *End = AddRec->evaluateAtIteration(BECount, *SE);
-    const SCEV *Diff0 = SE->getMinusSCEV(Start, Size);
-    const SCEV *Diff1 = SE->getMinusSCEV(End, Size);
-
-    // If the value of Step is non-negative and the AddRec is non-wrap, it
-    // reaches its maximum at the last iteration. So it's enouth to check
-    // whether End - Size is negative.
-    if (SE->isKnownNonNegative(Step) && SE->isKnownNegative(Diff1))
-      return true;
-
-    // If the value of Step is non-positive and the AddRec is non-wrap, the
-    // initial value is its maximum.
-    if (SE->isKnownNonPositive(Step) && SE->isKnownNegative(Diff0))
-      return true;
-
-    // Even if we don't know the sign of Step, either Start or End must be
-    // the maximum value of the AddRec since it is non-wrap.
-    if (SE->isKnownNegative(Diff0) && SE->isKnownNegative(Diff1))
-      return true;
-
-    return false;
-  };
-
-  if (CheckAddRecBECount())
-    return true;
-
-  // Check using normal isKnownNegative
-  const SCEV *LimitedBound = SE->getMinusSCEV(S, Size);
-  return SE->isKnownNegative(LimitedBound);
-}
-
 bool llvm::validateDelinearizationResult(ScalarEvolution &SE,
                                          ArrayRef<const SCEV *> Sizes,
                                          ArrayRef<const SCEV *> Subscripts,
@@ -782,7 +715,13 @@ bool llvm::validateDelinearizationResult(ScalarEvolution &SE,
     const SCEV *Subscript = Subscripts[I];
     if (!isKnownNonNegative(&SE, Subscript, Ptr))
       return false;
-    if (!isKnownLessThan(&SE, Subscript, Size))
+
+    // TODO: It may be better that delinearization itself unifies the types of
+    // all elements in Sizes and Subscripts.
+    Type *WiderTy = SE.getWiderType(Subscript->getType(), Size->getType());
+    Subscript = SE.getNoopOrSignExtend(Subscript, WiderTy);
+    Size = SE.getNoopOrSignExtend(Size, WiderTy);
+    if (!SE.isKnownPredicate(ICmpInst::ICMP_SLT, Subscript, Size))
       return false;
   }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/171821