[PATCH] D135837: [mlir][MemRef] Simplify extract_strided_metadata(reinterpret_cast)

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:670
+/// base, baseOffset, ... = extract_strided_metadata(src)
+/// offset = baseOffset + srcOffset
+/// sizes = srcSizes
----------------
nicolasvasilache wrote:
> This feels very fishy .. I would expect just srcOffset here.
> 
> Computing the sum is a "subview"-like semantics, reinterpret_cast should just specify the offset UIAM
That makes sense.
I thought it was adding say offset to the given src_memref, but indeed that's not really "reinterpret-y".

Fixing.


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:725
+
+    results[1] = makeComposedFoldedAffineApply(
+        rewriter, loc, s0 + s1,
----------------
nicolasvasilache wrote:
> I am really not sure about this ..
You're right, this is plain wrong :).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135837