[PATCH] D122857: [LoopCacheAnalysis] Enable delinearization of fixed sized arrays

Wed Apr 6 01:10:15 PDT 2022

nikic added inline comments.

================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:335
+  SmallVector<int, 4> SrcSizes;
+  getIndexExpressionsFromGEP(*SE, SrcGEP, SrcSubscripts, SrcSizes);
+
----------------
Meinersbur wrote:
> nikic wrote:
> > So, I guess this is something of a pre-existing issue, but this API looks completely broken to me. Deriving information from a GEP source element type is blatantly illegal under IR semantics -- in particular, the array bounds specified therein may be completely arbitrary.
> > 
> > The correct way to derive such information is from SCEV addrec expressions. For example in your first test you have `%arrayidx10` with SCEV `{{(8 + %a)<nuw>,+,8192}<nuw><%for.body>,+,4}<nuw><%for.body4>`, and the steps of those addrecs //are// semantically meaningful.
> It should be only a heuristic, using the fact that these indices are usually something sensible from the source code. In case of DependenceInfo, there is an additional check whether the derived indices are actually legal (unless `--da-disable-delinearization-checks=true`). LoopCacheAnalysis is an heuristic analysis entirely, deriving and heuristic cost function without a claim of semantics.
> 
> The SCEV addrec approach is already used by LoopCacheAnalysis in the form of `llvm::delinearize`.
> 
Okay, this is slightly less bad than I though then -- but GEP type structure should still not be used for heuristic purposes either.

If the SCEV addrec approach is already used in llvm::delinearize(), then why is this additional code needed? The addrecs involved in the motivational test cases contain all the necessary information, so there should be no need to look at GEP structure.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122857/new/

https://reviews.llvm.org/D122857