[Delinearization] Remove isKnownNonNegative (#171817)

Delinearization has its own `isKnownNonNegative` function, which wraps
`ScalarEvolution::isKnownNonNegative` and adds additional logic. The
additional logic is that, for a pointer addrec `{a,+,b}`, if the pointer
has `inbounds` and both `a` and `b` are known to be non-negative, then
the addrec is also known non-negative (i.e., it doesn't wrap). This
reasoning is incorrect. If the GEP and/or load/store using the pointer
are not unconditionally executed in the loop, then the addrec can still
wrap. Even though no actual example has been found where this causes a
miscompilation (probably because the subsequent checks fail so the
validation also fails), simply replacing it with
`ScalarEvolution::isKnownNonNegative` is safer, especially it doesn't
cause any regressions in the existing tests.

Resolve #169811

Author: kasuga-fj

llvm/include/llvm/Analysis/Delinearization.h

@@ -143,12 +143,10 @@ bool delinearizeFixedSizeArray(ScalarEvolution &SE, const SCEV *Expr,
 
 /// Check that each subscript in \p Subscripts is within the corresponding size
 /// in \p Sizes. For the outermost dimension, the subscript being negative is
-/// allowed. If \p Ptr is not nullptr, it may be used to get information from
-/// the IR pointer value, which may help in the validation.
+/// allowed.
 bool validateDelinearizationResult(ScalarEvolution &SE,
                                    ArrayRef<const SCEV *> Sizes,
-                                   ArrayRef<const SCEV *> Subscripts,
-                                   const Value *Ptr = nullptr);
+                                   ArrayRef<const SCEV *> Subscripts);
 
 /// Gathers the individual index expressions from a GEP instruction.
 ///

llvm/lib/Analysis/Delinearization.cpp

@@ -666,26 +666,6 @@ bool llvm::delinearizeFixedSizeArray(ScalarEvolution &SE, const SCEV *Expr,
   return !Subscripts.empty();
 }
 
-static bool isKnownNonNegative(ScalarEvolution *SE, const SCEV *S,
-                               const Value *Ptr) {
-  bool Inbounds = false;
-  if (auto *SrcGEP = dyn_cast<GetElementPtrInst>(Ptr))
-    Inbounds = SrcGEP->isInBounds();
-  if (Inbounds) {
-    if (const SCEVAddRecExpr *AddRec = dyn_cast<SCEVAddRecExpr>(S)) {
-      if (AddRec->isAffine()) {
-        // We know S is for Ptr, the operand on a load/store, so doesn't wrap.
-        // If both parts are NonNegative, the end result will be NonNegative
-        if (SE->isKnownNonNegative(AddRec->getStart()) &&
-            SE->isKnownNonNegative(AddRec->getOperand(1)))
-          return true;
-      }
-    }
-  }
-
-  return SE->isKnownNonNegative(S);
-}
-
 /// Compare to see if S is less than Size, using
 ///
 ///    isKnownNegative(S - Size)
@@ -755,8 +735,7 @@ static bool isKnownLessThan(ScalarEvolution *SE, const SCEV *S,
 
 bool llvm::validateDelinearizationResult(ScalarEvolution &SE,
                                          ArrayRef<const SCEV *> Sizes,
-                                         ArrayRef<const SCEV *> Subscripts,
-                                         const Value *Ptr) {
+                                         ArrayRef<const SCEV *> Subscripts) {
   // Sizes and Subscripts are as follows:
   //
   //   Sizes:      [UNK][S_2]...[S_n]
@@ -780,7 +759,7 @@ bool llvm::validateDelinearizationResult(ScalarEvolution &SE,
   for (size_t I = 1; I < Sizes.size(); ++I) {
     const SCEV *Size = Sizes[I - 1];
     const SCEV *Subscript = Subscripts[I];
-    if (!isKnownNonNegative(&SE, Subscript, Ptr))
+    if (!SE.isKnownNonNegative(Subscript))
       return false;
     if (!isKnownLessThan(&SE, Subscript, Size))
       return false;
@@ -943,8 +922,7 @@ void printDelinearization(raw_ostream &O, Function *F, LoopInfo *LI,
         O << "[" << *Subscripts[i] << "]";
       O << "\n";
 
-      bool IsValid = validateDelinearizationResult(
-          *SE, Sizes, Subscripts, getLoadStorePointerOperand(&Inst));
+      bool IsValid = validateDelinearizationResult(*SE, Sizes, Subscripts);
       O << "Delinearization validation: " << (IsValid ? "Succeeded" : "Failed")
         << "\n";
   }

llvm/lib/Analysis/DependenceAnalysis.cpp

@@ -3287,8 +3287,8 @@ bool DependenceInfo::tryDelinearizeFixedSize(
   // iff the subscripts are positive and are less than the range of the
   // dimension.
   if (!DisableDelinearizationChecks) {
-    if (!validateDelinearizationResult(*SE, SrcSizes, SrcSubscripts, SrcPtr) ||
-        !validateDelinearizationResult(*SE, DstSizes, DstSubscripts, DstPtr)) {
+    if (!validateDelinearizationResult(*SE, SrcSizes, SrcSubscripts) ||
+        !validateDelinearizationResult(*SE, DstSizes, DstSubscripts)) {
       SrcSubscripts.clear();
       DstSubscripts.clear();
       return false;
@@ -3307,8 +3307,6 @@ bool DependenceInfo::tryDelinearizeParametricSize(
     const SCEV *DstAccessFn, SmallVectorImpl<const SCEV *> &SrcSubscripts,
     SmallVectorImpl<const SCEV *> &DstSubscripts) {
 
-  Value *SrcPtr = getLoadStorePointerOperand(Src);
-  Value *DstPtr = getLoadStorePointerOperand(Dst);
   const SCEVUnknown *SrcBase =
       dyn_cast<SCEVUnknown>(SE->getPointerBase(SrcAccessFn));
   const SCEVUnknown *DstBase =
@@ -3353,8 +3351,8 @@ bool DependenceInfo::tryDelinearizeParametricSize(
   // FIXME: It may be better to record these sizes and add them as constraints
   // to the dependency checks.
   if (!DisableDelinearizationChecks)
-    if (!validateDelinearizationResult(*SE, Sizes, SrcSubscripts, SrcPtr) ||
-        !validateDelinearizationResult(*SE, Sizes, DstSubscripts, DstPtr))
+    if (!validateDelinearizationResult(*SE, Sizes, SrcSubscripts) ||
+        !validateDelinearizationResult(*SE, Sizes, DstSubscripts))
       return false;
 
   return true;