[llvm-branch-commits] [mlir] [mlir] Fold traverseRegionGraph into isRegionReachable (NFC) (PR #234162)

[Henrich Lauko via llvm-branch-commits]

https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/234162

>From 913f62e37362e08185ba646407701facbd17d26b Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Wed, 21 Oct 2026 08:06:29 +0000
Subject: [PATCH] [mlir] Fold traverseRegionGraph into isRegionReachable (NFC)

traverseRegionGraph had one caller, and that caller's stop condition only
compared against the target region. Inline it into isRegionReachable as a
plain worklist search. Drop the "has no parent op" error path, which cannot
fire because every region on the worklist comes from the op's own
getSuccessorRegions. insideMutuallyExclusiveRegions now returns early when
both ops are in the same region instead of running both searches first.

Assisted-by: Claude Code (Opus 5.5).
---
 mlir/lib/Interfaces/ControlFlowInterfaces.cpp | 82 +++++--------------
 .../Interfaces/ControlFlowInterfacesTest.cpp  | 14 ++++
 2 files changed, 35 insertions(+), 61 deletions(-)

diff --git a/mlir/lib/Interfaces/ControlFlowInterfaces.cpp b/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
index 3f4f5d7691df4..770d7c7b1aaca 100644
--- a/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
+++ b/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
@@ -247,71 +247,35 @@ static SmallVector<Region *> getSuccessorRegionsOf(Region *region) {
   return filterRegionSuccessors(successors);
 }
 
-/// Stop condition for `traverseRegionGraph`. The traversal is interrupted if
-/// this function returns "true" for a successor region.
-using StopConditionFn = function_ref<bool(Region *)>;
-
-/// Traverse the region graph starting at `begin`. The traversal is interrupted
-/// if `stopCondition` evaluates to "true" for a successor region. In that case,
-/// this function returns "true". Otherwise, if the traversal was not
-/// interrupted, this function returns "false".
-static bool traverseRegionGraph(Region *begin,
-                                StopConditionFn stopConditionFn) {
-  auto op = cast<RegionBranchOpInterface>(begin->getParentOp());
-  LDBG() << "Starting region graph traversal from region #"
-         << begin->getRegionNumber() << " in operation " << op->getName();
-
-  SmallVector<bool> visited(op->getNumRegions(), false);
-  visited[begin->getRegionNumber()] = true;
-  LDBG() << "Initialized visited array with " << op->getNumRegions()
-         << " regions";
+/// Return `true` if region `r` is reachable from region `begin` by taking at
+/// least one branch. So `begin` reaches itself only if it is on a cycle.
+static bool isRegionReachable(Region *begin, Region *r) {
+  assert(begin->getParentOp() == r->getParentOp() &&
+         "expected that both regions belong to the same op");
+  LDBG() << "Checking if region #" << r->getRegionNumber()
+         << " is reachable from region #" << begin->getRegionNumber();
 
+  SmallVector<bool> visited(begin->getParentOp()->getNumRegions(), false);
+  visited[begin->getRegionNumber()] = true;
   SmallVector<Region *> worklist = getSuccessorRegionsOf(begin);
-  LDBG() << "Initial worklist size: " << worklist.size();
-
-  // Process all regions in the worklist via DFS.
   while (!worklist.empty()) {
     Region *nextRegion = worklist.pop_back_val();
-    LDBG() << "Processing region #" << nextRegion->getRegionNumber()
-           << " from worklist (remaining: " << worklist.size() << ")";
-
-    // Check the stop condition before the visited set. `begin` starts out
-    // visited, and reaching it again must still stop the traversal.
-    if (stopConditionFn(nextRegion)) {
-      LDBG() << "Stop condition met for region #"
-             << nextRegion->getRegionNumber() << ", returning true";
+    // Check `r` before the visited set. `begin` starts out visited, and
+    // reaching it again must still count.
+    if (nextRegion == r) {
+      LDBG() << "Region #" << r->getRegionNumber() << " is reachable";
       return true;
     }
-    if (!nextRegion->getParentOp()) {
-      llvm::errs() << "Region " << *nextRegion << " has no parent op\n";
-      return false;
-    }
-    if (visited[nextRegion->getRegionNumber()]) {
-      LDBG() << "Region #" << nextRegion->getRegionNumber()
-             << " already visited, skipping";
+    if (visited[nextRegion->getRegionNumber()])
       continue;
-    }
     visited[nextRegion->getRegionNumber()] = true;
-    LDBG() << "Marking region #" << nextRegion->getRegionNumber()
-           << " as visited";
     llvm::append_range(worklist, getSuccessorRegionsOf(nextRegion));
   }
 
-  LDBG() << "Traversal completed, returning false";
+  LDBG() << "Region #" << r->getRegionNumber() << " is not reachable";
   return false;
 }
 
-/// Return `true` if region `r` is reachable from region `begin` according to
-/// the RegionBranchOpInterface (by taking a branch).
-static bool isRegionReachable(Region *begin, Region *r) {
-  assert(begin->getParentOp() == r->getParentOp() &&
-         "expected that both regions belong to the same op");
-  return traverseRegionGraph(begin, [&](Region *nextRegion) {
-    // Interrupt traversal if `r` was reached.
-    return nextRegion == r;
-  });
-}
-
 /// Return `true` if `a` and `b` are in mutually exclusive regions.
 ///
 /// 1. Find the first common of `a` and `b` (ancestor) that implements
@@ -364,16 +328,12 @@ bool mlir::insideMutuallyExclusiveRegions(Operation *a, Operation *b) {
 
     // `a` and `b` are in mutually exclusive regions if both regions are
     // distinct and neither region is reachable from the other region.
-    bool regionsAreDistinct = (regionA != regionB);
-    bool aNotReachableFromB = !isRegionReachable(regionA, regionB);
-    bool bNotReachableFromA = !isRegionReachable(regionB, regionA);
-
-    LDBG() << "Regions distinct: " << regionsAreDistinct
-           << ", A not reachable from B: " << aNotReachableFromB
-           << ", B not reachable from A: " << bNotReachableFromA;
-
-    bool mutuallyExclusive =
-        regionsAreDistinct && aNotReachableFromB && bNotReachableFromA;
+    if (regionA == regionB) {
+      LDBG() << "Operations are in the same region";
+      return false;
+    }
+    bool mutuallyExclusive = !isRegionReachable(regionA, regionB) &&
+                             !isRegionReachable(regionB, regionA);
     LDBG() << "Operations are mutually exclusive: " << mutuallyExclusive;
 
     return mutuallyExclusive;
diff --git a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp b/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
index df581d481644f..4546472ded592 100644
--- a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
+++ b/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
@@ -383,6 +383,20 @@ TEST_F(RegionGraphTest, DiamondIsNotALoop) {
   EXPECT_FALSE(op.hasLoop());
 }
 
+TEST_F(RegionGraphTest, DiamondReachability) {
+  RegionBranchOpInterface op = createRegionGraphOp(
+      /*entries=*/{0}, /*successors=*/{{1, 2}, {3}, {3}, {}});
+  auto terminator = [&](unsigned index) {
+    return &op->getRegion(index).front().front();
+  };
+  for (unsigned i = 0; i < 4; ++i)
+    EXPECT_FALSE(op.isRepetitiveRegion(i)) << "region " << i;
+  EXPECT_TRUE(insideMutuallyExclusiveRegions(terminator(1), terminator(2)));
+  EXPECT_TRUE(insideMutuallyExclusiveRegions(terminator(2), terminator(1)));
+  EXPECT_FALSE(insideMutuallyExclusiveRegions(terminator(0), terminator(3)));
+  EXPECT_FALSE(insideMutuallyExclusiveRegions(terminator(3), terminator(1)));
+}
+
 TEST_F(RegionGraphTest, LoopAfterEntryRegion) {
   RegionBranchOpInterface op =
       createRegionGraphOp(/*entries=*/{0}, /*successors=*/{{1}, {1}});