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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopCacheAnalysis.h:146-147
+
+  /// True if the underlying array is not parametric-sized.
+  bool IsFixedSize = false;
 };
----------------
Meinersbur wrote:
> In principle, each dimension can be dynamic or fixed size separately. For instance using C99 VLAs:
> ```
> void func(int n, double A[][128][n]);
> ```
> has 3 dimensions of which only the middle one is fixed size. However, DependenceInfo may not support them either?
The existing delinearization algorithms only work if all sizes are parametric or if all are constant. `A` in the example above will be a plain `double* ` pointer, so `getIndexExpressionsFromGEP()` won't be able to recover much from it. 


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:355
+
+  for (unsigned i = 1; i < Subscripts.size(); i++)
+    Sizes.push_back(SE->getConstant(Subscripts[i]->getType(), SrcSizes[i - 1]));
----------------
can use a range-based loop here


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:358
+
+  assert(SrcSubscripts.size() == SrcSizes.size() + 1 &&
+         "Expected equal number of entries in the list of size and "
----------------
[nit] this assert should be done before the copy into Sizes.


================
Comment at: llvm/lib/Analysis/LoopCacheAnalysis.cpp:392
+    if (tryDelinearizeFixedSize(&SE, &StoreOrLoadInst, AccessFn, Subscripts)) {
+      IsFixedSize = true;
+      /// The last element of \p Sizes is the element size.
----------------
Why do we need to store `IsFixedSize` as a data member?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122857