[PATCH] D77485: [MLIR] Handle in-place folding properly in greedy pattern rewrite driver
Uday Bondhugula via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 10 00:30:16 PDT 2020
bondhugula added inline comments.
================
Comment at: mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp:191
// Try to fold this op.
- if (succeeded(folder.tryToFold(op, collectOps, preReplaceAction))) {
+ bool inPlaceUpdate;
+ if ((succeeded(folder.tryToFold(op, collectOps, preReplaceAction,
----------------
rriddle wrote:
> We could just set this inside of preReplaceAction instead of changing the folder API for now?
Two quick questions
1) preReplaceAction is also not called when the folding fails. Did you mean to set 'foldedAndErased = true' in preReplaceAction? Leaving inPlaceUpdate to true when preReplaceAction is not called would be incorrect.
2) Also, wouldn't a tryToFold client want to know if the op was erased? I had a use case for that but that'll anyway not be needed once the child revision which adds 'op local fold and apply patterns' is available. But otherwise there would be no way to know if the fold erased the op.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77485/new/
https://reviews.llvm.org/D77485
More information about the llvm-commits
mailing list