Page MenuHomePhabricator

[mlir] Add `ReshapeMemrefCastOp` to Standard.
AcceptedPublic

Authored by pifon2a on Jul 2 2020, 11:39 AM.

Details

Reviewers
herhut
ftynse
Summary

The op allows to reshape memrefs using a 1D memref of extents. The shape memref
can also be dynamic.

Diff Detail

Event Timeline

pifon2a created this revision.Jul 2 2020, 11:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 2 2020, 11:39 AM

Can you make sure there is a thread on discourse before adding ops to the standard dialect?

herhut added a comment.Jul 3 2020, 2:01 AM

Thanks, some comments...

mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
1839

Please keep the file sorted.

1875

Why is unranked to unranked not supported?

mlir/lib/Dialect/StandardOps/IR/Ops.cpp
1911

Nit: You could also use isa<UnrankedMemRefType> here as you do not need the value.

1915

Nit: This is not the rank but the length of the shape operand.

1919

Maybe cannot use shape operand with dynamic length to cast ...?

1922

Maybe length of shape operand differs from the result's memref rank?

Can you make sure there is a thread on discourse before adding ops to the standard dialect?

https://llvm.discourse.group/t/standardops-add-reshapememrefcastop-to-standard/1350/2

pifon2a marked 8 inline comments as done.Jul 3 2020, 8:01 AM

Thank you!

mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
1875

I really need to see a use case for that. We can always relax this requirement later.

mlir/lib/Dialect/StandardOps/IR/Ops.cpp
1915

which is the rank of the resulting memref.

pifon2a updated this revision to Diff 275402.Jul 3 2020, 8:03 AM
pifon2a marked 2 inline comments as done.

Addressed the comments.

pifon2a updated this revision to Diff 275543.Jul 5 2020, 1:51 AM

Remove verification for unranked -> unranked memrefs.

herhut accepted this revision.Jul 6 2020, 3:43 AM
This revision is now accepted and ready to land.Jul 6 2020, 3:43 AM
pifon2a updated this revision to Diff 275665.Jul 6 2020, 4:45 AM

x index -> x i32