[delinearize] use SCEV exprs in getIndexExpressionsFromGEP

[sebpop]

clean up interface of getIndexExpressionsFromGEP to get SCEV expressions instead of int for Sizes of the arrays.
This intends to simplify the code in #156342 by avoiding conversions from SCEV to int and back to SCEV.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Sebastian Pop (sebpop)

Changes

clean up interface of getIndexExpressionsFromGEP to get SCEV expressions instead of int for Sizes of the arrays.
This intends to simplify the code in #156342 by avoiding conversions from SCEV to int and back to SCEV.

---

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

5 Files Affected:

• (modified) llvm/include/llvm/Analysis/Delinearization.h (+7-7)
• (modified) llvm/lib/Analysis/Delinearization.cpp (+4-3)
• (modified) llvm/lib/Analysis/DependenceAnalysis.cpp (+12-15)
• (modified) llvm/lib/Analysis/LoopCacheAnalysis.cpp (+2-3)
• (modified) polly/lib/Analysis/ScopBuilder.cpp (+3-7)

diff --git a/llvm/include/llvm/Analysis/Delinearization.h b/llvm/include/llvm/Analysis/Delinearization.h
index 434cfb61699d6..9c729b5cf18c8 100644
--- a/llvm/include/llvm/Analysis/Delinearization.h
+++ b/llvm/include/llvm/Analysis/Delinearization.h
@@ -145,15 +145,15 @@ bool delinearizeFixedSizeArray(ScalarEvolution &SE, const SCEV *Expr,
 ///
 /// This function optimistically assumes the GEP references into a fixed size
 /// array. If this is actually true, this function returns a list of array
-/// subscript expressions in \p Subscripts and a list of integers describing
-/// the size of the individual array dimensions in \p Sizes. Both lists have
-/// either equal length or the size list is one element shorter in case there
-/// is no known size available for the outermost array dimension. Returns true
-/// if successful and false otherwise.
+/// subscript expressions in \p Subscripts and a list of SCEV expressions
+/// describing the size of the individual array dimensions in \p Sizes. Both
+/// lists have either equal length or the size list is one element shorter in
+/// case there is no known size available for the outermost array dimension.
+/// Returns true if successful and false otherwise.
 bool getIndexExpressionsFromGEP(ScalarEvolution &SE,
                                 const GetElementPtrInst *GEP,
                                 SmallVectorImpl<const SCEV *> &Subscripts,
-                                SmallVectorImpl<int> &Sizes);
+                                SmallVectorImpl<const SCEV *> &Sizes);
 
 /// Implementation of fixed size array delinearization. Try to delinearize
 /// access function for a fixed size multi-dimensional array, by deriving
@@ -164,7 +164,7 @@ bool getIndexExpressionsFromGEP(ScalarEvolution &SE,
 bool tryDelinearizeFixedSizeImpl(ScalarEvolution *SE, Instruction *Inst,
                                  const SCEV *AccessFn,
                                  SmallVectorImpl<const SCEV *> &Subscripts,
-                                 SmallVectorImpl<int> &Sizes);
+                                 SmallVectorImpl<const SCEV *> &Sizes);
 
 struct DelinearizationPrinterPass
     : public PassInfoMixin<DelinearizationPrinterPass> {
diff --git a/llvm/lib/Analysis/Delinearization.cpp b/llvm/lib/Analysis/Delinearization.cpp
index 4064b25d9d4e7..f438a9bd9ecdd 100644
--- a/llvm/lib/Analysis/Delinearization.cpp
+++ b/llvm/lib/Analysis/Delinearization.cpp
@@ -659,7 +659,7 @@ bool llvm::delinearizeFixedSizeArray(ScalarEvolution &SE, const SCEV *Expr,
 bool llvm::getIndexExpressionsFromGEP(ScalarEvolution &SE,
                                       const GetElementPtrInst *GEP,
                                       SmallVectorImpl<const SCEV *> &Subscripts,
-                                      SmallVectorImpl<int> &Sizes) {
+                                      SmallVectorImpl<const SCEV *> &Sizes) {
   assert(Subscripts.empty() && Sizes.empty() &&
          "Expected output lists to be empty on entry to this function.");
   assert(GEP && "getIndexExpressionsFromGEP called with a null GEP");
@@ -690,7 +690,8 @@ bool llvm::getIndexExpressionsFromGEP(ScalarEvolution &SE,
 
     Subscripts.push_back(Expr);
     if (!(DroppedFirstDim && i == 2))
-      Sizes.push_back(ArrayTy->getNumElements());
+      Sizes.push_back(SE.getConstant(Expr->getType(),
+                                      ArrayTy->getNumElements()));
 
     Ty = ArrayTy->getElementType();
   }
@@ -706,7 +707,7 @@ bool llvm::getIndexExpressionsFromGEP(ScalarEvolution &SE,
 
 bool llvm::tryDelinearizeFixedSizeImpl(
     ScalarEvolution *SE, Instruction *Inst, const SCEV *AccessFn,
-    SmallVectorImpl<const SCEV *> &Subscripts, SmallVectorImpl<int> &Sizes) {
+    SmallVectorImpl<const SCEV *> &Subscripts, SmallVectorImpl<const SCEV *> &Sizes) {
   Value *SrcPtr = getLoadStorePointerOperand(Inst);
 
   // Check the simple case where the array dimensions are fixed size.
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 8d20b0e10305b..7dd0bea75c847 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -3504,8 +3504,8 @@ bool DependenceInfo::tryDelinearizeFixedSize(
            "expected src and dst scev unknowns to be equal");
   });
 
-  SmallVector<int, 4> SrcSizes;
-  SmallVector<int, 4> DstSizes;
+  SmallVector<const SCEV *, 4> SrcSizes;
+  SmallVector<const SCEV *, 4> DstSizes;
   if (!tryDelinearizeFixedSizeImpl(SE, Src, SrcAccessFn, SrcSubscripts,
                                    SrcSizes) ||
       !tryDelinearizeFixedSizeImpl(SE, Dst, DstAccessFn, DstSubscripts,
@@ -3513,7 +3513,7 @@ bool DependenceInfo::tryDelinearizeFixedSize(
     return false;
 
   // Check that the two size arrays are non-empty and equal in length and
-  // value.
+  // value.  SCEV expressions are uniqued, so we can compare pointers.
   if (SrcSizes.size() != DstSizes.size() ||
       !std::equal(SrcSizes.begin(), SrcSizes.end(), DstSizes.begin())) {
     SrcSubscripts.clear();
@@ -3535,7 +3535,7 @@ bool DependenceInfo::tryDelinearizeFixedSize(
   // iff the subscripts are positive and are less than the range of the
   // dimension.
   if (!DisableDelinearizationChecks) {
-    auto AllIndicesInRange = [&](SmallVector<int, 4> &DimensionSizes,
+    auto AllIndicesInRange = [&](SmallVector<const SCEV *, 4> &DimensionSizes,
                                  SmallVectorImpl<const SCEV *> &Subscripts,
                                  Value *Ptr) {
       size_t SSize = Subscripts.size();
@@ -3548,17 +3548,14 @@ bool DependenceInfo::tryDelinearizeFixedSize(
           });
           return false;
         }
-        if (auto *SType = dyn_cast<IntegerType>(S->getType())) {
-          const SCEV *Range = SE->getConstant(
-              ConstantInt::get(SType, DimensionSizes[I - 1], false));
-          if (!isKnownLessThan(S, Range)) {
-            LLVM_DEBUG({
-              dbgs() << "Check failed: !isKnownLessThan(S, Range)\n";
-              dbgs() << "  S: " << *S << "\n"
-                     << "  Range: " << *Range << "\n";
-            });
-            return false;
-          }
+        const SCEV *Range = DimensionSizes[I - 1];
+        if (!isKnownLessThan(S, Range)) {
+          LLVM_DEBUG({
+            dbgs() << "Check failed: !isKnownLessThan(S, Range)\n";
+            dbgs() << "  S: " << *S << "\n"
+                   << "  Range: " << *Range << "\n";
+          });
+          return false;
         }
       }
       return true;
diff --git a/llvm/lib/Analysis/LoopCacheAnalysis.cpp b/llvm/lib/Analysis/LoopCacheAnalysis.cpp
index 050c32707596a..74bcce8f51a4c 100644
--- a/llvm/lib/Analysis/LoopCacheAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopCacheAnalysis.cpp
@@ -356,15 +356,14 @@ CacheCostTy IndexedReference::computeRefCost(const Loop &L,
 
 bool IndexedReference::tryDelinearizeFixedSize(
     const SCEV *AccessFn, SmallVectorImpl<const SCEV *> &Subscripts) {
-  SmallVector<int, 4> ArraySizes;
+  SmallVector<const SCEV *, 4> ArraySizes;
   if (!tryDelinearizeFixedSizeImpl(&SE, &StoreOrLoadInst, AccessFn, Subscripts,
                                    ArraySizes))
     return false;
 
   // Populate Sizes with scev expressions to be used in calculations later.
   for (auto Idx : seq<unsigned>(1, Subscripts.size()))
-    Sizes.push_back(
-        SE.getConstant(Subscripts[Idx]->getType(), ArraySizes[Idx - 1]));
+    Sizes.push_back(ArraySizes[Idx - 1]);
 
   LLVM_DEBUG({
     dbgs() << "Delinearized subscripts of fixed-size array\n"
diff --git a/polly/lib/Analysis/ScopBuilder.cpp b/polly/lib/Analysis/ScopBuilder.cpp
index 67a4c43455809..ec77b15ddd768 100644
--- a/polly/lib/Analysis/ScopBuilder.cpp
+++ b/polly/lib/Analysis/ScopBuilder.cpp
@@ -1463,7 +1463,7 @@ bool ScopBuilder::buildAccessMultiDimFixed(MemAccInst Inst, ScopStmt *Stmt) {
     return false;
 
   SmallVector<const SCEV *, 4> Subscripts;
-  SmallVector<int, 4> Sizes;
+  SmallVector<const SCEV *, 4> Sizes;
   getIndexExpressionsFromGEP(SE, GEP, Subscripts, Sizes);
   auto *BasePtr = GEP->getOperand(0);
 
@@ -1475,8 +1475,6 @@ bool ScopBuilder::buildAccessMultiDimFixed(MemAccInst Inst, ScopStmt *Stmt) {
   if (BasePtr != BasePointer->getValue())
     return false;
 
-  std::vector<const SCEV *> SizesSCEV;
-
   const InvariantLoadsSetTy &ScopRIL = scop->getRequiredInvariantLoads();
 
   Loop *SurroundingLoop = Stmt->getSurroundingLoop();
@@ -1494,11 +1492,9 @@ bool ScopBuilder::buildAccessMultiDimFixed(MemAccInst Inst, ScopStmt *Stmt) {
   if (Sizes.empty())
     return false;
 
+  std::vector<const SCEV *> SizesSCEV;
   SizesSCEV.push_back(nullptr);
-
-  for (auto V : Sizes)
-    SizesSCEV.push_back(SE.getSCEV(
-        ConstantInt::get(IntegerType::getInt64Ty(BasePtr->getContext()), V)));
+  SizesSCEV.insert(SizesSCEV.end(), Sizes.begin(), Sizes.end());
 
   addArrayAccess(Stmt, Inst, AccType, BasePointer->getValue(), ElementType,
                  true, Subscripts, SizesSCEV, Val);

[github-actions]

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

[kasuga-fj]

Generally LGTM. Please apply clang-format.

[kasuga-fj]

I don't think the declaration needs to be moved.

[sjoerdmeijer]

With @kasuga-fj's minor comments addressed, LGTM too.