This is an archive of the discontinued LLVM Phabricator instance.

[mlir][shape] Fold shape.broadcast with scalar operands
ClosedPublic

Authored by herhut on Jul 15 2020, 12:27 AM.

Diff Detail

Event Timeline

herhut created this revision.Jul 15 2020, 12:27 AM
Herald added a reviewer: silvas. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
jpienaar accepted this revision.Jul 15 2020, 8:45 AM

Nice, thanks

This revision is now accepted and ready to land.Jul 15 2020, 8:45 AM
tpopp added inline comments.Jul 15 2020, 8:45 AM
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
70

If you're willing to add even more changes, this should be commutative.

93

Nit: Include in the description that you added assemblyFormats for these ops.

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

Nit: Change commit description to say "Fold shape.broadcast with at least 1 scalar operand" or similar since I read that as both being scalar.

mlir/test/Dialect/Shape/canonicalize.mlir
74

This test would have behaved the same before your changes because both are constant. I think the example above with the order of inputs flipped would currently fail.

tpopp accepted this revision.Jul 15 2020, 8:50 AM

LGTM otherwise

herhut updated this revision to Diff 278209.Jul 15 2020, 8:56 AM
herhut marked 2 inline comments as done.

Comments

herhut marked 4 inline comments as done.Jul 15 2020, 8:58 AM
herhut added inline comments.
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
70
mlir/test/Dialect/Shape/canonicalize.mlir
74

I added the flipped case.

herhut updated this revision to Diff 278228.Jul 15 2020, 9:49 AM
herhut marked an inline comment as done.

Add back test and unbreak.

This revision was automatically updated to reflect the committed changes.