[Mlir-commits] [mlir] [mlir][Analysis][NFC] Improve `RegionBranchOpInterface` API usage (PR #173983)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

Remove a helper function and query the `RegionBranchOpInterface` instead. (Which does the same thing.) Also add a TODO for a bug in the implementation of `SliceWalk.cpp`. (The bug is not fixed yet.)


---
Full diff: https://github.com/llvm/llvm-project/pull/173983.diff


1 Files Affected:

- (modified) mlir/lib/Analysis/SliceWalk.cpp (+10-50) 


``````````diff
diff --git a/mlir/lib/Analysis/SliceWalk.cpp b/mlir/lib/Analysis/SliceWalk.cpp
index 863f260cd4b6a..8760a9f6d8721 100644
--- a/mlir/lib/Analysis/SliceWalk.cpp
+++ b/mlir/lib/Analysis/SliceWalk.cpp
@@ -32,52 +32,6 @@ WalkContinuation mlir::walkSlice(ValueRange rootValues,
   return WalkContinuation::skip();
 }
 
-/// Returns the operands from all predecessor regions that match `operandNumber`
-/// for the `successor` region within `regionOp`.
-static SmallVector<Value>
-getRegionPredecessorOperands(RegionBranchOpInterface regionOp,
-                             RegionSuccessor successor,
-                             unsigned operandNumber) {
-  SmallVector<Value> predecessorOperands;
-
-  // Returns true if `successors` contains `successor`.
-  auto isContained = [](ArrayRef<RegionSuccessor> successors,
-                        RegionSuccessor successor) {
-    auto *it = llvm::find_if(successors, [&successor](RegionSuccessor curr) {
-      return curr.getSuccessor() == successor.getSuccessor();
-    });
-    return it != successors.end();
-  };
-
-  // Search the operand ranges on the region operation itself.
-  SmallVector<Attribute> operandAttributes(regionOp->getNumOperands());
-  SmallVector<RegionSuccessor> successors;
-  regionOp.getEntrySuccessorRegions(operandAttributes, successors);
-  if (isContained(successors, successor)) {
-    OperandRange operands = regionOp.getEntrySuccessorOperands(successor);
-    predecessorOperands.push_back(operands[operandNumber]);
-  }
-
-  // Search the operand ranges on region terminators.
-  for (Region &region : regionOp->getRegions()) {
-    for (Block &block : region) {
-      auto terminatorOp =
-          dyn_cast<RegionBranchTerminatorOpInterface>(block.getTerminator());
-      if (!terminatorOp)
-        continue;
-      SmallVector<Attribute> operandAttributes(terminatorOp->getNumOperands());
-      SmallVector<RegionSuccessor> successors;
-      terminatorOp.getSuccessorRegions(operandAttributes, successors);
-      if (isContained(successors, successor)) {
-        OperandRange operands = terminatorOp.getSuccessorOperands(successor);
-        predecessorOperands.push_back(operands[operandNumber]);
-      }
-    }
-  }
-
-  return predecessorOperands;
-}
-
 /// Returns the predecessor branch operands that match `blockArg`, or nullopt if
 /// some of the predecessor terminators do not implement the BranchOpInterface.
 static std::optional<SmallVector<Value>>
@@ -115,8 +69,11 @@ mlir::getControlFlowPredecessors(Value value) {
       return std::nullopt;
     // Add the control flow predecessor operands to the work list.
     RegionSuccessor region(regionOp, regionOp->getResults());
-    SmallVector<Value> predecessorOperands = getRegionPredecessorOperands(
-        regionOp, region, opResult.getResultNumber());
+    SmallVector<Value> predecessorOperands;
+    // TODO: This assumes that there are no non-forwarded op results in front
+    // of the forwarded op results.
+    regionOp.getPredecessorValues(region, opResult.getResultNumber(),
+                                  predecessorOperands);
     return predecessorOperands;
   }
 
@@ -127,8 +84,11 @@ mlir::getControlFlowPredecessors(Value value) {
     if (auto regionBranchOp =
             dyn_cast<RegionBranchOpInterface>(block->getParentOp())) {
       RegionSuccessor region(blockArg.getParentRegion());
-      SmallVector<Value> predecessorOperands = getRegionPredecessorOperands(
-          regionBranchOp, region, blockArg.getArgNumber());
+      SmallVector<Value> predecessorOperands;
+      // TODO: This assumes that there are no non-forwarded block arguments in
+      // front of the forwarded block arguments.
+      regionBranchOp.getPredecessorValues(region, blockArg.getArgNumber(),
+                                          predecessorOperands);
       return predecessorOperands;
     }
     // If the interface is not implemented, there are no control flow

``````````

</details>


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