[PATCH] D77083: [MLIR] Add pattern rewriter util to erase block; add affine.if pattern to remove dead else blocks

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula created this revision.
bondhugula added reviewers: jpienaar, rriddle, mehdi_amini.
Herald added subscribers: llvm-commits, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, burmako.
Herald added 1 blocking reviewer(s): rriddle.
Herald added a project: LLVM.
bondhugula retitled this revision from "[MLIR] Add pattern rewriter util to erase block; remove dead else" to "[MLIR] Add pattern rewriter util to erase block; add affine.if pattern to remove dead else blocks".
bondhugula edited the summary of this revision.
rriddle requested changes to this revision.
rriddle added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/include/mlir/IR/PatternMatch.h:285
+  /// known to have no uses.
+  virtual void eraseBlock(Block *block);
+
----------------
Can you add proper handling for DialectConversion?


================
Comment at: mlir/lib/Dialect/Affine/IR/AffineOps.cpp:1577
+    if (ifOp.elseRegion().empty() ||
+        ifOp.getElseBlock()->getOperations().size() > 1)
+      return failure();
----------------
Please use has_single_element instead. size() is O(N).


================
Comment at: mlir/lib/Dialect/Affine/IR/AffineOps.cpp:1580
+
+    rewriter.startRootUpdate(ifOp);
+    rewriter.eraseBlock(ifOp.getElseBlock());
----------------
nit: 
```
rewriter.updateRootInPlace(ifOp, [&] {
  rewriter.eraseBlock(if.getElseBlock());
});
```

?


================
Comment at: mlir/lib/IR/PatternMatch.cpp:94
+  for (auto &op : llvm::make_early_inc_range(*block)) {
+    assert(op.use_empty() && "expected 'op' to have no uses");
+    notifyOperationRemoved(&op);
----------------
This seems wrong, any non-trivially empty block will likely fail this check. Did you want to iterate the block in reverse?


================
Comment at: mlir/lib/IR/PatternMatch.cpp:96
+    notifyOperationRemoved(&op);
+    op.erase();
+  }
----------------
Why not just replace this loop with 'eraseOp'?


Add a pattern rewriter utility to erase blocks (while notifying the
pattern rewriting driver of the erased ops). Use this to remove trivial
else blocks in affine.if ops.

Signed-off-by: Uday Bondhugula <uday at polymagelabs.com>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77083

Files:
  mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
  mlir/include/mlir/IR/PatternMatch.h
  mlir/lib/Dialect/Affine/IR/AffineOps.cpp
  mlir/lib/IR/PatternMatch.cpp
  mlir/test/Transforms/canonicalize.mlir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77083.253670.patch
Type: text/x-patch
Size: 4631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200330/02cc486c/attachment-0001.bin>