[PATCH] D77487: [MLIR] Introduce applyOpPatternsAndFold for op local rewrites

Wed Apr 15 02:08:45 PDT 2020

bondhugula added inline comments.

================
Comment at: mlir/include/mlir/IR/PatternMatch.h:464
+/// apply any patterns recursively to the regions of `op`.
+bool applyOpPatternsAndFold(Operation *op,
+                            const OwningRewritePatternList &patterns,
----------------
rriddle wrote:
> I'm of the opinion that we should remove the return value from these methods. It is not used anywhere, and it isn't clear what the user would use it for in any of the existing usages.
The child revision has the use case where it's essential - let's discuss there if there are alternatives.

================
Comment at: mlir/lib/Dialect/Affine/Transforms/SimplifyAffineStructures.cpp:95
+    if (isa<AffineForOp>(op))
+      AffineForOp::getCanonicalizationPatterns(patterns, op->getContext());
+    else if (isa<AffineIfOp>(op))
----------------
rriddle wrote:
> Same here. Can we avoid rebuilding a pattern set for each operation?
Sure - done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77487