This is an archive of the discontinued LLVM Phabricator instance.

[Mlir] Implement printer, parser, verifier and builder for shape.reduce.
ClosedPublic

Authored by pifon2a on Jun 4 2020, 12:21 PM.

Details

Summary

Adding the missing pieces for shape.reduce to make it usable and readable.

Diff Detail

Event Timeline

pifon2a created this revision.Jun 4 2020, 12:21 PM
Herald added a reviewer: silvas. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
pifon2a updated this revision to Diff 268564.Jun 4 2020, 12:34 PM

Remove parentheses.

pifon2a edited the summary of this revision. (Show Details)
silvas accepted this revision.Jun 4 2020, 1:41 PM

LGTM with one requested modification and a nit.

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

can we make "lci" a bit more descriptive?

mlir/test/Dialect/Shape/invalid.mlir
39

Can you verify the yield as well?

frgossen accepted this revision.Jun 5 2020, 1:23 AM
pifon2a marked 2 inline comments as done.Jun 5 2020, 2:18 AM
pifon2a added inline comments.
mlir/test/Dialect/Shape/invalid.mlir
39

This will become a part of YieldOp verification. I will submit with a separate PR when cleaning-up "YieldOp".

pifon2a updated this revision to Diff 268715.Jun 5 2020, 2:18 AM

Address a comment.

pifon2a marked an inline comment as done and an inline comment as not done.Jun 5 2020, 2:19 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 5 2020, 2:43 AM
This revision was automatically updated to reflect the committed changes.
jpienaar added inline comments.Jun 5 2020, 12:05 PM
mlir/lib/Dialect/Shape/IR/Shape.cpp
511

The error reported with emitOpError will already start with the op (so you'd have an error like "'shape.ReduceOp' op ReduceOp body")