[Mlir-commits] [mlir] [mlir] Add a utility method to move operation dependencies. (PR #129975)

[llvmlistbot at llvm.org]

================
@@ -1054,3 +1057,65 @@ LogicalResult mlir::simplifyRegions(RewriterBase &rewriter,
   return success(eliminatedBlocks || eliminatedOpsOrArgs ||
                  mergedIdenticalBlocks || droppedRedundantArguments);
 }
+
+//===---------------------------------------------------------------------===//
+// Move operation dependencies
+//===---------------------------------------------------------------------===//
+
+LogicalResult mlir::moveOperationDependencies(RewriterBase &rewriter,
+                                              Operation *op,
+                                              Operation *insertionPoint,
+                                              DominanceInfo &dominance) {
+  // Currently unsupported case where the op and insertion point are
+  // in different basic blocks.
+  if (op->getBlock() != insertionPoint->getBlock()) {
+    return rewriter.notifyMatchFailure(
+        op, "unsupported caes where operation and insertion point are not in "
+            "the sme basic block");
+  }
+
+  // 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`
+  BackwardSliceOptions options;
+  options.inclusive = true;
+  options.filter = [&](Operation *sliceBoundaryOp) {
+    return !dominance.properlyDominates(sliceBoundaryOp, insertionPoint);
+  };
+  llvm::SetVector<Operation *> slice;
+
+  // Get the defined slice for operands.
+  for (Value operand : op->getOperands()) {
+    getBackwardSlice(operand, &slice, options);
+  }
+  auto regions = op->getRegions();
+  if (!regions.empty()) {
----------------
MaheshRavishankar wrote:

Good suggestion on this, but let me clarify something 
```
func.func @move_region_dependencies() -> f32 {  
  %0 = "before"() : () -> (f32)  
  %1 = "moved_op"() ({  
    "yield"(%0) : (f32) -> ()  
  }) : () -> (f32)  
  %2 = "foo"() ({  
    "yield"(%1) : (f32) -> ()  
  }) : () -> (f32)  
  return %2 : f32  
}  
```
Here the operations shouldnt be moved. `"foo"` is directly dependent on `%0 = "before"`. So moving all dependencies before would be illegal and so the method does nothing (it returns a failure). But your suggestion does work for an example like this
```
func.func @move_region_dependencies() -> f32 {  
  %0 = "before"() : () -> (f32)  
  %1 = "moved_op_1"() : () -> (f32)
  %2 = "moved_op"() ({  
    "yield"(%1) : (f32) -> ()  
  }) : () -> (f32)  
  %3 = "foo"() ({  
    "yield"(%2) : (f32) -> ()  
  }) : () -> (f32)  
  return %3 : f32  
}  
```
Here `"foo"` does not directly depend on `%0 = "before"` and you can move all its dependencies. I added this as a test.

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