[PATCH] D80189: [mlir] Add canonicalization for Cstr and Assuming Shape Ops.

Wed May 20 19:21:02 PDT 2020

silvas resigned from this revision.
silvas added a comment.

Looks like this has been shraded into a couple other patches. Resigning from this master one as I reviewed the others.

================
Comment at: mlir/lib/Dialect/Shape/IR/Shape.cpp:186
+// Removes AssumingOp with a passing condition and inlines the region.
+struct AssumingWithTrue : public OpRewritePattern<AssumingOp> {
+  using OpRewritePattern<AssumingOp>::OpRewritePattern;
----------------
tpopp wrote:
> silvas wrote:
> > We might not want this pattern always. For example, if we have a pass that does code motion on islands, this canonicalization can prematurely remove the island which will make that pass unable to operate properly.  Let's hold off on adding it until we have a compelling use case.
> I don't find this a compelling argument. Not all code will be in islands and these islands are for the purpose of making code conditional on some assertion. If it's statically known that the assertion is not a problem and the island cannot be removed, then the island is being misused for another purpose and blocking possible compiler optimizations.
Good point!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80189/new/

https://reviews.llvm.org/D80189