[Mlir-commits] [mlir] [mlir] Add side-effect check to moveOperationDependencies (PR #176361)

[Jorn Tuyls]

================
@@ -1115,15 +1115,24 @@ LogicalResult mlir::moveOperationDependencies(RewriterBase &rewriter,
 
   // Find the backward slice of operation for each `Value` the operation
   // depends on. Prune the slice to only include operations not already
-  // dominated by the `insertionPoint`
+  // dominated by the `insertionPoint`.
   BackwardSliceOptions options;
   options.inclusive = false;
   options.omitUsesFromAbove = false;
   // Since current support is to only move within a same basic block,
   // the slices dont need to look past block arguments.
   options.omitBlockArguments = true;
+  bool dependsOnSideEffectingOp = false;
   options.filter = [&](Operation *sliceBoundaryOp) {
-    return !dominance.properlyDominates(sliceBoundaryOp, insertionPoint);
+    // Skip the root op - we're moving its dependencies, not the op itself.
+    // The root op is filtered out by options.inclusive = false anyway.
+    if (sliceBoundaryOp == op)
+      return true;
+    bool mustMove =
+        !dominance.properlyDominates(sliceBoundaryOp, insertionPoint);
+    if (mustMove && !isPure(sliceBoundaryOp))
----------------
jtuyls wrote:

Yes, I now updated both to stop traversal in this case.

https://github.com/llvm/llvm-project/pull/176361