[PATCH] D77485: [MLIR] Handle in-place folding properly in greedy pattern rewrite driver

[Uday Bondhugula via Phabricator via llvm-commits]

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