[PATCH] D108885: [Delinerization] Keep array element byte offset.

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

> The makes the returned Subscripts array one larger than the Sizes array. This actually would be expected, a subscript each size plus one subscript for the division remainder representing the outermost dimension of unknown size.

The Size array includes the element size, so it already makes up for the lack of a value for the outermost dimension.

Instead of treating the element offset as a subscript, could we make `computeAccessFunctions` return the offset (if non-zero) in a separate output parameter?
Or maybe we should consider delinearization unsuccessful when there is a non-zero remainder after the last real subscript is already computed.



================
Comment at: llvm/include/llvm/Analysis/ScalarEvolution.h:1201
   ///
-  /// Overall, we have: A[][n][m], and the access function: A[j+k][2i][5i].
+  /// Overall, for the address of a memory access, we have: A[][n][m][4], and
+  /// the access function: A[j+k][2i][5i][0] where [4] is the array element size
----------------
address of a memory access -> address of the above memory access


================
Comment at: llvm/lib/Analysis/DependenceAnalysis.cpp:3454
   // Fail when there is only a subscript: that's a linearized access function.
-  if (SrcSubscripts.size() < 2 || DstSubscripts.size() < 2 ||
+  if (SrcSubscripts.size() <= 2 || DstSubscripts.size() <= 2 ||
       SrcSubscripts.size() != DstSubscripts.size())
----------------
I'm worried that having an extra subscript could confuse the analysis. Each separable subscript typically corresponds to a distinct loop level (unless the subscript is ZIV), but if we include the element offset as a "subscript" it would normally not have any corresponding loop levels.


================
Comment at: llvm/test/Analysis/Delinearization/byte_offset.ll:10
+; CHECK: ArrayRef[0][{(6 + (4 * (sext i16 %i401 to i64))<nsw>)<nsw>,+,1}<%for.body.i4567>][((sext i32 %mul7.i4534 to i64) + {(sext i32 %i1 to i64),+,((sext i32 (1 + ((1 + %shl.i.i) * (1 + %shl.i.i)) + %shl.i.i) to i64) * (sext i32 %i1 to i64))}<%for.body703>)]
+
+%struct.S1 = type { %struct.S1*, i8*, i16, i16, i16, i16 }
----------------
could we have the psudo-c for this IR added here as a comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108885