This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Minor cleanup for Shape dialect.
ClosedPublic

Authored by jurahul on Dec 8 2020, 5:50 PM.

Details

Summary
  • Remove some unused types from the Shape dialect
  • Fix from_extent_tensor to only allow 1D index tensors
  • Fix assuming_yield to only allow shape.assuming as the parent op.
  • Fix some documentation typos and reword some things.

Diff Detail

Event Timeline

jurahul created this revision.Dec 8 2020, 5:50 PM
jurahul requested review of this revision.Dec 8 2020, 5:50 PM
jurahul retitled this revision from [MLIR] Minor cleanup for Shape dialect. - Remove some unused types from the Shape dialect - Fix from_extent_tensor to only allow 1D index tensors - Fix assuming_yield to only allow shape.assuming as the parent op. - Fix some documentation typos... to [MLIR] Minor cleanup for Shape dialect. .Dec 8 2020, 5:51 PM
jurahul edited the summary of this revision. (Show Details)
silvas accepted this revision.Dec 8 2020, 7:35 PM
silvas added inline comments.
mlir/test/Dialect/Shape/invalid.mlir
160

we generally don't explicitly test things verified by ODS predicates

jurahul added inline comments.Dec 8 2020, 8:31 PM
mlir/test/Dialect/Shape/invalid.mlir
160

Makes sense. I'll remove this test.

jurahul updated this revision to Diff 310414.Dec 8 2020, 8:33 PM
jurahul retitled this revision from [MLIR] Minor cleanup for Shape dialect. to [MLIR] Minor cleanup for Shape dialect..
jurahul edited the summary of this revision. (Show Details)

Remove test case

jpienaar accepted this revision.Dec 9 2020, 6:43 AM

Thanks. Yes the element type we have no tests for yet and can reintroduce it when we do.

mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
331

While here, the first one is a must, the second is a may. E.g., for the first one where we need to represent errors we can't use index, but the second one we could use size still. I think in the code we already allow this to allow local folding without needing to do full propagation.

This revision is now accepted and ready to land.Dec 9 2020, 6:43 AM
jurahul updated this revision to Diff 310563.Dec 9 2020, 9:23 AM

Address feedback

mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
331

Done. I also added similar language for shape.add as well.

This revision was landed with ongoing or failed builds.Dec 9 2020, 2:22 PM
This revision was automatically updated to reflect the committed changes.