This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][Shape] Add conversion for missing ops to standard
ClosedPublic

Authored by herhut on Jul 28 2020, 5:17 AM.

Details

Summary

This adds conversions for const_size and to_extent_tensor. Also, cast-like operations are now folded away if the source and target types are the same.

Diff Detail

Event Timeline

herhut created this revision.Jul 28 2020, 5:17 AM
Herald added a reviewer: silvas. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
herhut updated this revision to Diff 281201.Jul 28 2020, 5:44 AM

Remove a test

herhut published this revision for review.Jul 28 2020, 5:44 AM
herhut added inline comments.Jul 28 2020, 5:49 AM
mlir/lib/Dialect/Shape/IR/Shape.cpp
543

I don't know how to test this case, as it is illegal. I just added it for completeness if this every materializes during some conversion.

silvas added inline comments.Jul 28 2020, 9:18 AM
mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp
133

nit: notifyMatchFailure

mlir/lib/Dialect/Shape/IR/Shape.cpp
543

I think this would do it:

func @f(%arg0: index) {

%0 = size_to_index(%arg0)
return s

}

foldCastOp is for non-constant but where we otherwise know the op is structurally identity.

frgossen added inline comments.Jul 28 2020, 9:35 AM
mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp
67

Should we make this pattern fail in case the result type is a shape.size?

herhut added inline comments.Jul 28 2020, 10:00 AM
mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp
67

But even if the result is a shape.size, we would lower it to a index value when going to standard. It cannot be an error.

mlir/lib/Dialect/Shape/IR/Shape.cpp
543

If I write a test case where %arg0 is of type index, then I cannot even parse it because passing and index to this operation is invalid. I guess I should remove it again. I also have no good use for it.

This op is the structural identity of the two types are the same. Would you prefer to spell this out here?

silvas accepted this revision.Jul 28 2020, 3:20 PM
silvas added inline comments.
mlir/lib/Dialect/Shape/IR/Shape.cpp
543

Sounds like it can be a TODO for when size_to_index accepts index inputs.

jpienaar accepted this revision.Jul 28 2020, 3:35 PM
jpienaar added inline comments.
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
438

Ouch, thanks :)

This revision is now accepted and ready to land.Jul 28 2020, 3:35 PM
This revision was landed with ongoing or failed builds.Jul 29 2020, 4:05 AM
This revision was automatically updated to reflect the committed changes.