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

Unit TestsFailed

TimeTest
680 mswindows > LLVM.tools/yaml2obj/ELF/DWARF::Unknown Unit Message ("")
Script: -- : 'RUN: at line 7'; c:\ws\w4\llvm-project\premerge-checks\build\bin\yaml2obj.exe --docnum=1 -DENDIAN=ELFDATA2LSB C:\ws\w4\llvm-project\premerge-checks\llvm\test\tools\yaml2obj\ELF\DWARF\debug-gnu-pubnames.yaml -o C:\ws\w4\llvm-project\premerge-checks\build\test\tools\yaml2obj\ELF\DWARF\Output\debug-gnu-pubnames.yaml.tmp1.le.o
740 mswindows > LLVM.tools/yaml2obj/ELF/DWARF::Unknown Unit Message ("")
Script: -- : 'RUN: at line 7'; c:\ws\w4\llvm-project\premerge-checks\build\bin\yaml2obj.exe --docnum=1 -DENDIAN=ELFDATA2LSB C:\ws\w4\llvm-project\premerge-checks\llvm\test\tools\yaml2obj\ELF\DWARF\debug-gnu-pubtypes.yaml -o C:\ws\w4\llvm-project\premerge-checks\build\test\tools\yaml2obj\ELF\DWARF\Output\debug-gnu-pubtypes.yaml.tmp1.le.o

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
1837

Please keep the file sorted.

1873

Why is unranked to unranked not supported?

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

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

1910

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

1914

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

1917

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
1873

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

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

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