[PATCH] D77083: [MLIR] Add pattern rewriter util to erase block; add affine.if pattern to remove dead else blocks
River Riddle via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 30 13:04:37 PDT 2020
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'?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77083/new/
https://reviews.llvm.org/D77083
More information about the llvm-commits
mailing list