[PATCH] D76268: [MLIR] Fix op folding to not run pre-replace when not constant folding

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula created this revision.
bondhugula added reviewers: rriddle, mehdi_amini, jpienaar.
Herald added subscribers: llvm-commits, Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, burmako.
Herald added a project: LLVM.

OperationFolder::tryToFold was running the pre-replacement action even
when there was no constant folding, i.e., when the operation was just
being updated in place but was not going to be replaced. This led to
nested ops being unnecessarily removed from the worklist and only being
processed in the next outer iteration of the greedy pattern rewriter,
which is also why this didn't affect the final output IR but only the
convergence rate. It also led to an op's results' users to be
unnecessarily added to the worklist.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76268

Files:
  mlir/lib/Transforms/Utils/FoldUtils.cpp


Index: mlir/lib/Transforms/Utils/FoldUtils.cpp
===================================================================
--- mlir/lib/Transforms/Utils/FoldUtils.cpp
+++ mlir/lib/Transforms/Utils/FoldUtils.cpp
@@ -85,17 +85,17 @@
   if (failed(tryToFold(op, results, processGeneratedConstants)))
     return failure();
 
-  // Constant folding succeeded. We will start replacing this op's uses and
-  // eventually erase this op. Invoke the callback provided by the caller to
-  // perform any pre-replacement action.
-  if (preReplaceAction)
-    preReplaceAction(op);
-
   // Check to see if the operation was just updated in place.
   if (results.empty())
     return success();
 
-  // Otherwise, replace all of the result values and erase the operation.
+  // Constant folding succeeded. We will start replacing this op's uses and
+  // erase this op. Invoke the callback provided by the caller to perform any
+  // pre-replacement action.
+  if (preReplaceAction)
+    preReplaceAction(op);
+
+  // Replace all of the result values and erase the operation.
   for (unsigned i = 0, e = results.size(); i != e; ++i)
     op->getResult(i).replaceAllUsesWith(results[i]);
   op->erase();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76268.250693.patch
Type: text/x-patch
Size: 1183 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200317/fc147faa/attachment-0001.bin>