Page MenuHomePhabricator

[mlir] Canonicalization of shape.cstr_broadcastable
ClosedPublic

Authored by tpopp on May 20 2020, 9:08 AM.

Details

Summary

This allows replacing of this op with a true_witness in the case of both
inputs being const_shapes and being found to be broadcastable.

Diff Detail

Event Timeline

tpopp created this revision.May 20 2020, 9:08 AM
Herald added a reviewer: silvas. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
silvas requested changes to this revision.May 20 2020, 11:37 AM
silvas added inline comments.
mlir/lib/Dialect/Shape/IR/Shape.cpp
274

As far as the constant folding logic, you can just use an i1 for a witness. You just need to add a case in ShapeDialect::materializeConstant know how to materialize it into a true_witness op or false_witness op (or just a single const_witness?).

Maybe you can rename this patch "introduce basic constant folding for witnesses" and use this fold as the first example?

This revision now requires changes to proceed.May 20 2020, 11:37 AM
herhut accepted this revision.May 25 2020, 4:49 AM

I am fine with this to land and make the transition to folding later.

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

Would a simple op.lhs() == op.rhs() work?

281

getOperand(0)->lhs()?
getOperand(1)->rhs()?

tpopp updated this revision to Diff 268478.Jun 4 2020, 8:10 AM
tpopp marked 4 inline comments as done.

Switch to folding and also handle repeated inputs.

Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2020, 8:10 AM
Herald added a subscriber: mgorny. · View Herald Transcript
tpopp added inline comments.Jun 4 2020, 8:11 AM
mlir/lib/Dialect/Shape/IR/Shape.cpp
274

It took me a long time to figure out that I could use a BoolAttr for this. In the spirit of small commits, I'm leaving the ConstWitnessOp creation separate, but I'm not folding this for constant false witnesses because that needs to be carefully thought through as that is removing an assert that can be triggered.

silvas accepted this revision.Jun 4 2020, 12:41 PM

Looks great! Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Jun 5 2020, 2:10 AM
This revision was automatically updated to reflect the committed changes.
isuruf added a subscriber: isuruf.Jun 5 2020, 10:18 AM

This commit breaks building MLIR for me. You need to create the directory IR in the build directory in mlir/lib/Dialect/Shape/CMakeLists.txt. Otherwise mlir-tblgen fails with a file not found error.