This is an archive of the discontinued LLVM Phabricator instance.

[mlir][MemRef] Simplify extract_strided_metadata(reinterpret_cast)
ClosedPublic

Authored by qcolombet on Oct 12 2022, 5:56 PM.

Details

Summary

This patch adds a pattern to simplify

base, offset, sizes, strides =
  extract_strided_metadata(
    reinterpret_cast(src, srcOffset, srcSizes, srcStrides))

Into

base, baseOffset, ... = extract_strided_metadata(src)
offset = srcOffset
sizes = srcSizes
strides = srcStrides

Note: Reinterpret_cast with unranked sources are not simplified since they cannot feed extract_strided_metadata operations.
Note2: This patch is part of a series that replaces the lowering of view like operations in MemRefToLLVM with composable patterns.
Note3: This patch depends on other patches in that series but only for the output of the tests.

Diff Detail

Event Timeline

qcolombet created this revision.Oct 12 2022, 5:56 PM
qcolombet requested review of this revision.Oct 12 2022, 5:56 PM
qcolombet added inline comments.Oct 12 2022, 6:15 PM
mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp
687

Is there a generic way of doing that?
Something like ExtractStridedMetadata::verifySourceType?

mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp
667

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

687

Implement your own ?

722

I am really not sure about this ..

qcolombet added inline comments.Oct 13 2022, 8:20 AM
mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp
667

That makes sense.
I thought it was adding say offset to the given src_memref, but indeed that's not really "reinterpret-y".

Fixing.

722

You're right, this is plain wrong :).

qcolombet edited the summary of this revision. (Show Details)
  • Just forward the source offset. Reinterpret_cast doesn't do any math.
qcolombet marked an inline comment as done.Oct 13 2022, 11:56 AM
qcolombet added inline comments.
mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp
667

Fixed.

687

I'll look into that as a follow up patch if that works for you, since if this doesn't exist yet, it will likely require some tablegen work.

qcolombet updated this revision to Diff 469745.Oct 21 2022, 1:29 PM
  • Use inferReturnTypes to check if an extract_strided_metadata operation can be built instead of hardcoding the MemRefType.
qcolombet marked an inline comment as done.Oct 21 2022, 1:31 PM
qcolombet added inline comments.
mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp
687

Done that part by reusing the inferReturnTypes API that we enabled for these operations at https://reviews.llvm.org/D135734.

nicolasvasilache accepted this revision.Oct 27 2022, 8:27 AM
This revision is now accepted and ready to land.Oct 27 2022, 8:27 AM
This revision was landed with ongoing or failed builds.Nov 14 2022, 10:42 AM
This revision was automatically updated to reflect the committed changes.