[Delinearization] Remove tryDelinearizeFixedSizeImpl

[kasuga-fj]

tryDelinearizeFixedSizeImpl is a heuristic function relying on GEP's type information. Using these information to drive an optimization heuristic is not allowed, so this function should be removed. As #161822 and #164798 have eliminated all calls to this, this patch removes the function itself.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Ryotaro Kasuga (kasuga-fj)

Changes

tryDelinearizeFixedSizeImpl is a heuristic function relying on GEP's type information. Using these information to drive an optimization heuristic is not allowed, so this function should be removed. As #161822 and #164798 have eliminated all calls to this, this patch removes the function itself.

---

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

2 Files Affected:

• (modified) llvm/include/llvm/Analysis/Delinearization.h (-11)
• (modified) llvm/lib/Analysis/Delinearization.cpp (-38)

diff --git a/llvm/include/llvm/Analysis/Delinearization.h b/llvm/include/llvm/Analysis/Delinearization.h
index 434cfb61699d6..cdcc3b67fccf1 100644
--- a/llvm/include/llvm/Analysis/Delinearization.h
+++ b/llvm/include/llvm/Analysis/Delinearization.h
@@ -155,17 +155,6 @@ bool getIndexExpressionsFromGEP(ScalarEvolution &SE,
                                 SmallVectorImpl<const SCEV *> &Subscripts,
                                 SmallVectorImpl<int> &Sizes);
 
-/// Implementation of fixed size array delinearization. Try to delinearize
-/// access function for a fixed size multi-dimensional array, by deriving
-/// subscripts from GEP instructions. Returns true upon success and false
-/// otherwise. \p Inst is the load/store instruction whose pointer operand is
-/// the one we want to delinearize. \p AccessFn is its corresponding SCEV
-/// expression w.r.t. the surrounding loop.
-bool tryDelinearizeFixedSizeImpl(ScalarEvolution *SE, Instruction *Inst,
-                                 const SCEV *AccessFn,
-                                 SmallVectorImpl<const SCEV *> &Subscripts,
-                                 SmallVectorImpl<int> &Sizes);
-
 struct DelinearizationPrinterPass
     : public PassInfoMixin<DelinearizationPrinterPass> {
   explicit DelinearizationPrinterPass(raw_ostream &OS);
diff --git a/llvm/lib/Analysis/Delinearization.cpp b/llvm/lib/Analysis/Delinearization.cpp
index 4064b25d9d4e7..8a8c2277012ec 100644
--- a/llvm/lib/Analysis/Delinearization.cpp
+++ b/llvm/lib/Analysis/Delinearization.cpp
@@ -704,44 +704,6 @@ bool llvm::getIndexExpressionsFromGEP(ScalarEvolution &SE,
   return !Subscripts.empty();
 }
 
-bool llvm::tryDelinearizeFixedSizeImpl(
-    ScalarEvolution *SE, Instruction *Inst, const SCEV *AccessFn,
-    SmallVectorImpl<const SCEV *> &Subscripts, SmallVectorImpl<int> &Sizes) {
-  Value *SrcPtr = getLoadStorePointerOperand(Inst);
-
-  // Check the simple case where the array dimensions are fixed size.
-  auto *SrcGEP = dyn_cast<GetElementPtrInst>(SrcPtr);
-  if (!SrcGEP)
-    return false;
-
-  getIndexExpressionsFromGEP(*SE, SrcGEP, Subscripts, Sizes);
-
-  // Check that the two size arrays are non-empty and equal in length and
-  // value.
-  // TODO: it would be better to let the caller to clear Subscripts, similar
-  // to how we handle Sizes.
-  if (Sizes.empty() || Subscripts.size() <= 1) {
-    Subscripts.clear();
-    return false;
-  }
-
-  // Check that for identical base pointers we do not miss index offsets
-  // that have been added before this GEP is applied.
-  Value *SrcBasePtr = SrcGEP->getOperand(0)->stripPointerCasts();
-  const SCEVUnknown *SrcBase =
-      dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFn));
-  if (!SrcBase || SrcBasePtr != SrcBase->getValue()) {
-    Subscripts.clear();
-    return false;
-  }
-
-  assert(Subscripts.size() == Sizes.size() + 1 &&
-         "Expected equal number of entries in the list of size and "
-         "subscript.");
-
-  return true;
-}
-
 namespace {
 
 void printDelinearization(raw_ostream &O, Function *F, LoopInfo *LI,

[github-actions]

🐧 Linux x64 Test Results

• 186454 tests passed
• 4865 tests skipped

[sjoerdmeijer]

Thanks, LGTM

@sebpop pointed one thing out to me offline: maybe Polly test cases need fixing up.
@Meinersbur: what is the policy around this? Should that be part of this patch? Or done separately?

[kasuga-fj]

Polly uses getIndexExpressionsFromGEP, which is different from tryDelinearizeFixedSizeImpl. So this PR should not affect Polly test cases. We also should remove getIndexExpressionsFromGEP in the future, and it needs to update Polly test cases, which Michael is working on (#164798 (review))

[sjoerdmeijer]

Polly uses getIndexExpressionsFromGEP, which is different from tryDelinearizeFixedSizeImpl. So this PR should not affect Polly test cases. We also should remove getIndexExpressionsFromGEP in the future, and it needs to update Polly test cases, which Michael is working on (#164798 (review))

Ok, great, then you can just go ahead and merge this I think.

[Meinersbur]

LGTM, Polly not affected (you would see that in the pre-merge CI failing)

Working on removing getIndexExpressionsFromGEP from Polly. The problem I encountered is that the matrix-multiplication pattern recognition relies on multi-dimensional accesses. Detecting the pattern with linearized accesses is not so easy.

[sebpop]

the matrix-multiplication pattern recognition relies on multi-dimensional accesses. Detecting the pattern with linearized accesses is not so easy.

I have fixed all Polly tests with reading the array info from global or alloca dimensions:
#156342

[Meinersbur]

I have fixed all Polly tests with reading the array info from global or alloca dimensions: #156342

I have a patch that adpts the tests to single-dimensional access functions, or if that does not work, use import-jscop to recover the multi-dimensional ones. The latter mostly affects the matmul detection which relies on the form of the access function.

This means that in practice, matmul optimization cannot be triggered with Clang-compiled programs anymore. With #156342, it theoretically can with global variables, but realistically I don't think there are programs out there that use globals for matrices sufficiently large (and not pass it as pointer to a function that implements matmul) to make matmul optimization worth it. If you know one e.g. somewhere on GitHub, feel free to point it out to me.

[kasuga-fj]

I don't know much about Polly so this maybe a silly question, but is it impossible to infer the mulidimensional array size from the access function? I imagine a very simple loop like this:

for (int i = 0; i < 100; i++)
  for (int j = 0; j < 200; j++)
    for (int k = 0; k < 300; k++)
      C[i][j] += A[i][k] * B[k][j];

I believe the existing delinearization functionality would be able to infer the sizes of the arrays A, B and C from the access patterns. Or Polly handles more complex cases like tiled loops?