[Mlir-commits] [mlir] [mlir] Fix false positive loops in RegionBranchOpInterface::hasLoop (PR #233784)
Henrich Lauko
llvmlistbot at llvm.org
Wed Oct 21 02:57:50 PDT 2026
https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/233784
>From 16010ab9b5786c84c69adc3e18e793eff2f47241 Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Tue, 20 Oct 2026 10:20:53 +0000
Subject: [PATCH 1/2] [mlir] Fix false positive loops in
RegionBranchOpInterface::hasLoop
hasLoop reported a loop whenever its traversal popped a region it had
already visited. The worklist gets one entry per successor of each
terminator, so a region reachable along two paths was popped twice even
without a cycle, for example when two terminators of a region branch to
the same successor region.
hasLoop now reports a loop only if a region reachable from the parent op
can reach itself, which is the check isRepetitiveRegion already does.
Drop the now unused visited argument of the traversal stop condition.
Assisted-by: Claude Code (Claude Opus 5.5)
---
mlir/lib/Interfaces/ControlFlowInterfaces.cpp | 53 ++++--------
.../Interfaces/ControlFlowInterfacesTest.cpp | 84 ++++++++++++++++++-
2 files changed, 99 insertions(+), 38 deletions(-)
diff --git a/mlir/lib/Interfaces/ControlFlowInterfaces.cpp b/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
index c2a2f7e2561096..060cb99b3a51c7 100644
--- a/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
+++ b/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
@@ -222,10 +222,8 @@ LogicalResult detail::verifyRegionBranchOpInterface(Operation *op) {
}
/// Stop condition for `traverseRegionGraph`. The traversal is interrupted if
-/// this function returns "true" for a successor region. The first parameter is
-/// the successor region. The second parameter indicates all already visited
-/// regions.
-using StopConditionFn = function_ref<bool(Region *, ArrayRef<bool> visited)>;
+/// 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,
@@ -280,7 +278,7 @@ static bool traverseRegionGraph(Region *begin,
LDBG() << "Processing region #" << nextRegion->getRegionNumber()
<< " from worklist (remaining: " << worklist.size() << ")";
- if (stopConditionFn(nextRegion, visited)) {
+ if (stopConditionFn(nextRegion)) {
LDBG() << "Stop condition met for region #"
<< nextRegion->getRegionNumber() << ", returning true";
return true;
@@ -309,11 +307,10 @@ static bool traverseRegionGraph(Region *begin,
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, ArrayRef<bool> visited) {
- // Interrupt traversal if `r` was reached.
- return nextRegion == r;
- });
+ 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.
@@ -405,35 +402,17 @@ bool RegionBranchOpInterface::hasLoop() {
LDBG() << "Checking if operation " << getOperation()->getName()
<< " has loops";
+ // There is a loop iff some region reachable from the parent op can reach
+ // itself. Reaching a region along several paths (e.g., from multiple
+ // terminators that branch to the same successor region) is not a loop.
SmallVector<RegionSuccessor> entryRegions;
getSuccessorRegions(RegionBranchPoint::parent(), entryRegions);
- LDBG() << "Found " << entryRegions.size() << " entry regions";
-
- for (RegionSuccessor successor : entryRegions) {
- if (successor.isRegion()) {
- LDBG() << "Checking entry region #"
- << successor.getSuccessor()->getRegionNumber() << " for loops";
-
- bool hasLoop =
- traverseRegionGraph(successor.getSuccessor(),
- [](Region *nextRegion, ArrayRef<bool> visited) {
- // Interrupt traversal if the region was already
- // visited.
- return visited[nextRegion->getRegionNumber()];
- });
-
- if (hasLoop) {
- LDBG() << "Found loop in entry region #"
- << successor.getSuccessor()->getRegionNumber();
- return true;
- }
- } else {
- LDBG() << "Skipping operation successor";
- }
- }
-
- LDBG() << "No loops found in operation";
- return false;
+ return llvm::any_of(entryRegions, [](RegionSuccessor successor) {
+ return successor.isRegion() &&
+ traverseRegionGraph(successor.getSuccessor(), [](Region *region) {
+ return isRegionReachable(region, region);
+ });
+ });
}
OperandRange
diff --git a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp b/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
index 5b5da0534393b0..d11764d87eb1a7 100644
--- a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
+++ b/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
@@ -132,12 +132,44 @@ struct SequentialRegionsOp
SequentialRegionsOp>::getSuccessorRegions;
};
+/// Region 0 is entered from the parent op and branches to region 1, which
+/// branches back to the parent op. With the `loop` attribute, region 1 also
+/// branches to itself.
+struct EntryRegionsOp
+ : public Op<EntryRegionsOp, RegionBranchOpInterface::Trait> {
+ using Op::Op;
+ static ArrayRef<StringRef> getAttributeNames() { return {}; }
+
+ static StringRef getOperationName() { return "cftest.entry_regions_op"; }
+
+ void getSuccessorRegions(RegionBranchPoint point,
+ SmallVectorImpl<RegionSuccessor> ®ions) {
+ Operation *op = getOperation();
+ if (point.isParent()) {
+ regions.push_back(RegionSuccessor(&op->getRegion(0)));
+ return;
+ }
+ Region *from = point.getTerminatorPredecessorOrNull()->getParentRegion();
+ if (from == &op->getRegion(0) || op->hasAttr("loop"))
+ regions.push_back(RegionSuccessor(&op->getRegion(1)));
+ if (from == &op->getRegion(1))
+ regions.push_back(RegionSuccessor(op));
+ }
+
+ ValueRange getSuccessorInputs(RegionSuccessor successor) {
+ return successor.isOperation() ? ValueRange(getOperation()->getResults())
+ : ValueRange();
+ }
+
+ using RegionBranchOpInterface::Trait<EntryRegionsOp>::getSuccessorRegions;
+};
+
/// A dialect putting all the above together.
struct CFTestDialect : Dialect {
explicit CFTestDialect(MLIRContext *ctx)
: Dialect(getDialectNamespace(), ctx, TypeID::get<CFTestDialect>()) {
addOperations<DummyOp, MutuallyExclusiveRegionsOp, LoopRegionsOp,
- DoubleLoopRegionsOp, SequentialRegionsOp>();
+ DoubleLoopRegionsOp, SequentialRegionsOp, EntryRegionsOp>();
}
static StringRef getDialectNamespace() { return "cftest"; }
};
@@ -284,3 +316,53 @@ TEST(RegionBranchOpInterface, NotRecursiveRegions) {
EXPECT_EQ(getEnclosingRepetitiveRegion(op1), nullptr);
EXPECT_EQ(getEnclosingRepetitiveRegion(op2), nullptr);
}
+
+TEST(RegionBranchOpInterface, MultipleExitsToSameRegionIsNotALoop) {
+ // Both terminators of region 0 branch to region 1. Region 1 is reached
+ // twice, but the region graph has no cycle.
+ const char *ir = R"MLIR(
+"cftest.entry_regions_op"() (
+ {
+ "cftest.dummy_op"() : () -> ()
+ ^bb1:
+ "cftest.dummy_op"() : () -> ()
+ },
+ {"cftest.dummy_op"() : () -> ()}
+ ) : () -> ()
+ )MLIR";
+
+ DialectRegistry registry;
+ registry.insert<CFTestDialect>();
+ MLIRContext ctx(registry);
+
+ OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(ir, &ctx);
+ ASSERT_TRUE(module);
+ auto regionOp = cast<RegionBranchOpInterface>(
+ &module->getBody()->getOperations().front());
+
+ EXPECT_FALSE(regionOp.isRepetitiveRegion(0));
+ EXPECT_FALSE(regionOp.isRepetitiveRegion(1));
+ EXPECT_FALSE(regionOp.hasLoop());
+}
+
+TEST(RegionBranchOpInterface, LoopAfterEntryRegion) {
+ const char *ir = R"MLIR(
+"cftest.entry_regions_op"() (
+ {"cftest.dummy_op"() : () -> ()},
+ {"cftest.dummy_op"() : () -> ()}
+ ) {loop} : () -> ()
+ )MLIR";
+
+ DialectRegistry registry;
+ registry.insert<CFTestDialect>();
+ MLIRContext ctx(registry);
+
+ OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(ir, &ctx);
+ ASSERT_TRUE(module);
+ auto regionOp = cast<RegionBranchOpInterface>(
+ &module->getBody()->getOperations().front());
+
+ EXPECT_FALSE(regionOp.isRepetitiveRegion(0));
+ EXPECT_TRUE(regionOp.isRepetitiveRegion(1));
+ EXPECT_TRUE(regionOp.hasLoop());
+}
>From 02a5ec97e42d39c48959d1027f47e4755e54aa44 Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Wed, 21 Oct 2026 07:36:16 +0000
Subject: [PATCH 2/2] [mlir] Detect hasLoop cycles with a single DFS
Calling isRegionReachable for every region the traversal popped made
hasLoop quadratic in the number of region successor edges. Use one
iterative depth-first search from the entry regions instead, in a new
canReachCycle helper. Reaching a region that is still on the DFS path
is a loop, reaching a finished one is not. Each region is queried at
most once, the search stops at the first loop, and the stack holds
one pointer-sized item per entry so small ops need no heap
allocation.
Both traversals now get a region's successors from
RegionBranchOpInterface::getSuccessorRegions(Region &) instead of
asking each terminator with null operand attributes. The in-tree
terminators that override getSuccessorRegions (scf.condition and
cir.condition) return the same regions as their parent op when no
operand is constant.
Restore the dropped debug output. Rename the multi-branch test and
replace its mock op with one whose region graph comes from attributes,
built by a test fixture. Add tests for a diamond, a loop
through the entry region, a loop in an unreachable region and
overlapping entry regions.
Assisted-by: Claude Code (Claude Opus 5.5)
---
mlir/lib/Interfaces/ControlFlowInterfaces.cpp | 128 +++++++++-----
.../Interfaces/ControlFlowInterfacesTest.cpp | 157 ++++++++++++------
2 files changed, 188 insertions(+), 97 deletions(-)
diff --git a/mlir/lib/Interfaces/ControlFlowInterfaces.cpp b/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
index 060cb99b3a51c7..9813206c95b236 100644
--- a/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
+++ b/mlir/lib/Interfaces/ControlFlowInterfaces.cpp
@@ -15,6 +15,7 @@
#include "mlir/IR/PatternMatch.h"
#include "mlir/Interfaces/ControlFlowInterfaces.h"
#include "llvm/ADT/EquivalenceClasses.h"
+#include "llvm/ADT/PointerIntPair.h"
#include "llvm/Support/DebugLog.h"
using namespace mlir;
@@ -221,6 +222,31 @@ LogicalResult detail::verifyRegionBranchOpInterface(Operation *op) {
return success();
}
+/// Return the regions among `successors`, dropping branches back to the
+/// parent op.
+static SmallVector<Region *>
+filterRegionSuccessors(ArrayRef<RegionSuccessor> successors) {
+ SmallVector<Region *> regions;
+ for (RegionSuccessor successor : successors)
+ if (successor.isRegion())
+ regions.push_back(successor.getSuccessor());
+ return regions;
+}
+
+/// Return the regions that the terminators of `region` may branch to.
+///
+/// This asks the parent op instead of each terminator. A terminator's own
+/// `getSuccessorRegions` only narrows the parent's answer based on constant
+/// operands, and no operand values are known here.
+static SmallVector<Region *> getSuccessorRegionsOf(Region *region) {
+ SmallVector<RegionSuccessor> successors;
+ cast<RegionBranchOpInterface>(region->getParentOp())
+ .getSuccessorRegions(*region, successors);
+ LDBG() << "Region #" << region->getRegionNumber() << " has "
+ << successors.size() << " successors";
+ 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 *)>;
@@ -240,36 +266,7 @@ static bool traverseRegionGraph(Region *begin,
LDBG() << "Initialized visited array with " << op->getNumRegions()
<< " regions";
- // Retrieve all successors of the region and enqueue them in the worklist.
- SmallVector<Region *> worklist;
- auto enqueueAllSuccessors = [&](Region *region) {
- LDBG() << "Enqueuing successors for region #" << region->getRegionNumber();
- SmallVector<Attribute> operandAttributes(op->getNumOperands());
- for (Block &block : *region) {
- if (block.empty())
- continue;
- auto terminator =
- dyn_cast<RegionBranchTerminatorOpInterface>(block.back());
- if (!terminator)
- continue;
- SmallVector<RegionSuccessor> successors;
- operandAttributes.resize(terminator->getNumOperands());
- terminator.getSuccessorRegions(operandAttributes, successors);
- LDBG() << "Found " << successors.size()
- << " successors from terminator in block";
- for (RegionSuccessor successor : successors) {
- if (successor.isRegion()) {
- worklist.push_back(successor.getSuccessor());
- LDBG() << "Added region #"
- << successor.getSuccessor()->getRegionNumber()
- << " to worklist";
- } else {
- LDBG() << "Skipping operation successor";
- }
- }
- }
- };
- enqueueAllSuccessors(begin);
+ SmallVector<Region *> worklist = getSuccessorRegionsOf(begin);
LDBG() << "Initial worklist size: " << worklist.size();
// Process all regions in the worklist via DFS.
@@ -278,6 +275,8 @@ static bool traverseRegionGraph(Region *begin,
LDBG() << "Processing region #" << nextRegion->getRegionNumber()
<< " from worklist (remaining: " << worklist.size() << ")";
+ // Check the stop condition before the visited set: `begin` is marked as
+ // visited, but reaching it again must still stop the traversal.
if (stopConditionFn(nextRegion)) {
LDBG() << "Stop condition met for region #"
<< nextRegion->getRegionNumber() << ", returning true";
@@ -295,7 +294,7 @@ static bool traverseRegionGraph(Region *begin,
visited[nextRegion->getRegionNumber()] = true;
LDBG() << "Marking region #" << nextRegion->getRegionNumber()
<< " as visited";
- enqueueAllSuccessors(nextRegion);
+ llvm::append_range(worklist, getSuccessorRegionsOf(nextRegion));
}
LDBG() << "Traversal completed, returning false";
@@ -398,21 +397,66 @@ bool RegionBranchOpInterface::isRepetitiveRegion(unsigned index) {
return isRepetitive;
}
+/// Return "true" if a cycle in the region graph is reachable from one of
+/// `entryRegions`, which must belong to the same op.
+///
+/// This is an iterative depth-first search. A region is on the DFS path from
+/// the time the search enters it until all regions reachable from it are
+/// finished. Reaching a region on the path closes a cycle. Reaching a finished
+/// region (e.g., from two terminators that branch to the same successor
+/// region) does not.
+static bool canReachCycle(ArrayRef<Region *> entryRegions) {
+ if (entryRegions.empty())
+ return false;
+ enum class State : uint8_t { Unvisited, OnPath, Finished };
+ SmallVector<State> states(
+ entryRegions.front()->getParentOp()->getNumRegions(), State::Unvisited);
+
+ // Each stack item either enters its region or, if the flag is set, leaves
+ // it. Entering a region pushes its leave item below the enter items of its
+ // successors, so the region stays on the path until they are all finished.
+ using StackItem = llvm::PointerIntPair<Region *, 1, bool>;
+ SmallVector<StackItem> stack;
+ for (Region *region : entryRegions)
+ stack.push_back(StackItem(region, /*isLeave=*/false));
+ while (!stack.empty()) {
+ StackItem item = stack.pop_back_val();
+ Region *region = item.getPointer();
+ bool isLeave = item.getInt();
+ State &state = states[region->getRegionNumber()];
+ if (isLeave) {
+ state = State::Finished;
+ continue;
+ }
+ if (state == State::OnPath) {
+ LDBG() << "Found loop through region #" << region->getRegionNumber();
+ return true;
+ }
+ if (state == State::Finished)
+ continue;
+
+ state = State::OnPath;
+ stack.push_back(StackItem(region, /*isLeave=*/true));
+ for (Region *successor : getSuccessorRegionsOf(region))
+ stack.push_back(StackItem(successor, /*isLeave=*/false));
+ }
+ return false;
+}
+
bool RegionBranchOpInterface::hasLoop() {
LDBG() << "Checking if operation " << getOperation()->getName()
<< " has loops";
- // There is a loop iff some region reachable from the parent op can reach
- // itself. Reaching a region along several paths (e.g., from multiple
- // terminators that branch to the same successor region) is not a loop.
- SmallVector<RegionSuccessor> entryRegions;
- getSuccessorRegions(RegionBranchPoint::parent(), entryRegions);
- return llvm::any_of(entryRegions, [](RegionSuccessor successor) {
- return successor.isRegion() &&
- traverseRegionGraph(successor.getSuccessor(), [](Region *region) {
- return isRegionReachable(region, region);
- });
- });
+ SmallVector<RegionSuccessor> entrySuccessors;
+ getSuccessorRegions(RegionBranchPoint::parent(), entrySuccessors);
+ SmallVector<Region *> entryRegions = filterRegionSuccessors(entrySuccessors);
+ LDBG() << "Found " << entryRegions.size() << " entry regions";
+
+ if (!canReachCycle(entryRegions)) {
+ LDBG() << "No loops found in operation";
+ return false;
+ }
+ return true;
}
OperandRange
diff --git a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp b/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
index d11764d87eb1a7..0ac3804afea4ce 100644
--- a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
+++ b/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "mlir/Interfaces/ControlFlowInterfaces.h"
+#include "mlir/IR/Builders.h"
#include "mlir/IR/BuiltinOps.h"
#include "mlir/IR/Dialect.h"
#include "mlir/IR/DialectImplementation.h"
@@ -132,28 +133,33 @@ struct SequentialRegionsOp
SequentialRegionsOp>::getSuccessorRegions;
};
-/// Region 0 is entered from the parent op and branches to region 1, which
-/// branches back to the parent op. With the `loop` attribute, region 1 also
-/// branches to itself.
-struct EntryRegionsOp
- : public Op<EntryRegionsOp, RegionBranchOpInterface::Trait> {
+/// The region graph of this op is given by attributes: `entries` lists the
+/// regions that are entered from the op, and `successors[i]` lists the regions
+/// that each terminator of region i branches to. Each terminator also branches
+/// back to the op.
+struct RegionGraphOp
+ : public Op<RegionGraphOp, RegionBranchOpInterface::Trait> {
using Op::Op;
static ArrayRef<StringRef> getAttributeNames() { return {}; }
- static StringRef getOperationName() { return "cftest.entry_regions_op"; }
+ static StringRef getOperationName() { return "cftest.region_graph_op"; }
void getSuccessorRegions(RegionBranchPoint point,
SmallVectorImpl<RegionSuccessor> ®ions) {
Operation *op = getOperation();
+ DenseI64ArrayAttr targets;
if (point.isParent()) {
- regions.push_back(RegionSuccessor(&op->getRegion(0)));
- return;
- }
- Region *from = point.getTerminatorPredecessorOrNull()->getParentRegion();
- if (from == &op->getRegion(0) || op->hasAttr("loop"))
- regions.push_back(RegionSuccessor(&op->getRegion(1)));
- if (from == &op->getRegion(1))
+ targets = cast<DenseI64ArrayAttr>(op->getAttr("entries"));
+ } else {
+ // Branching back to the op adds no edge between regions. It only lets
+ // every region exit.
regions.push_back(RegionSuccessor(op));
+ Region *from = point.getTerminatorPredecessorOrNull()->getParentRegion();
+ targets = cast<DenseI64ArrayAttr>(
+ cast<ArrayAttr>(op->getAttr("successors"))[from->getRegionNumber()]);
+ }
+ for (int64_t target : targets.asArrayRef())
+ regions.push_back(RegionSuccessor(&op->getRegion(target)));
}
ValueRange getSuccessorInputs(RegionSuccessor successor) {
@@ -161,7 +167,7 @@ struct EntryRegionsOp
: ValueRange();
}
- using RegionBranchOpInterface::Trait<EntryRegionsOp>::getSuccessorRegions;
+ using RegionBranchOpInterface::Trait<RegionGraphOp>::getSuccessorRegions;
};
/// A dialect putting all the above together.
@@ -169,7 +175,7 @@ struct CFTestDialect : Dialect {
explicit CFTestDialect(MLIRContext *ctx)
: Dialect(getDialectNamespace(), ctx, TypeID::get<CFTestDialect>()) {
addOperations<DummyOp, MutuallyExclusiveRegionsOp, LoopRegionsOp,
- DoubleLoopRegionsOp, SequentialRegionsOp, EntryRegionsOp>();
+ DoubleLoopRegionsOp, SequentialRegionsOp, RegionGraphOp>();
}
static StringRef getDialectNamespace() { return "cftest"; }
};
@@ -317,52 +323,93 @@ TEST(RegionBranchOpInterface, NotRecursiveRegions) {
EXPECT_EQ(getEnclosingRepetitiveRegion(op2), nullptr);
}
-TEST(RegionBranchOpInterface, MultipleExitsToSameRegionIsNotALoop) {
- // Both terminators of region 0 branch to region 1. Region 1 is reached
- // twice, but the region graph has no cycle.
- const char *ir = R"MLIR(
-"cftest.entry_regions_op"() (
- {
- "cftest.dummy_op"() : () -> ()
- ^bb1:
- "cftest.dummy_op"() : () -> ()
- },
- {"cftest.dummy_op"() : () -> ()}
- ) : () -> ()
- )MLIR";
+/// Builds `cftest.region_graph_op` ops and keeps them alive for the test.
+class RegionGraphTest : public ::testing::Test {
+protected:
+ RegionGraphTest() : builder(&ctx) {
+ ctx.loadDialect<CFTestDialect>();
+ module = ModuleOp::create(builder.getUnknownLoc());
+ builder.setInsertionPointToEnd(module->getBody());
+ }
- DialectRegistry registry;
- registry.insert<CFTestDialect>();
- MLIRContext ctx(registry);
+ /// Create a `cftest.region_graph_op` with one region per entry of
+ /// `successors`. Each region has one block, whose terminator branches to the
+ /// regions listed for it.
+ RegionBranchOpInterface
+ createRegionGraphOp(ArrayRef<int64_t> entries,
+ ArrayRef<SmallVector<int64_t>> successors) {
+ OperationState state(builder.getUnknownLoc(),
+ RegionGraphOp::getOperationName());
+ state.addAttribute("entries", builder.getDenseI64ArrayAttr(entries));
+ state.addAttribute(
+ "successors",
+ builder.getArrayAttr(llvm::map_to_vector(
+ successors, [&](ArrayRef<int64_t> regions) -> Attribute {
+ return builder.getDenseI64ArrayAttr(regions);
+ })));
+ for (size_t i = 0, e = successors.size(); i < e; ++i)
+ state.addRegion();
+ Operation *op = builder.create(state);
+ for (Region ®ion : op->getRegions())
+ addTerminatedBlock(region);
+ return cast<RegionBranchOpInterface>(op);
+ }
- OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(ir, &ctx);
- ASSERT_TRUE(module);
- auto regionOp = cast<RegionBranchOpInterface>(
- &module->getBody()->getOperations().front());
+ /// Append a block to `region` that holds only a region branch terminator.
+ void addTerminatedBlock(Region ®ion) {
+ OpBuilder::InsertionGuard guard(builder);
+ builder.createBlock(®ion);
+ builder.create(
+ OperationState(builder.getUnknownLoc(), DummyOp::getOperationName()));
+ }
+
+ MLIRContext ctx;
+ OpBuilder builder;
+ OwningOpRef<ModuleOp> module;
+};
- EXPECT_FALSE(regionOp.isRepetitiveRegion(0));
- EXPECT_FALSE(regionOp.isRepetitiveRegion(1));
- EXPECT_FALSE(regionOp.hasLoop());
+TEST_F(RegionGraphTest, MultipleBranchesToSameRegionIsNotALoop) {
+ RegionBranchOpInterface op =
+ createRegionGraphOp(/*entries=*/{0}, /*successors=*/{{1}, {}});
+ // Give region 0 a second terminator. Both terminators branch to region 1.
+ addTerminatedBlock(op->getRegion(0));
+ EXPECT_FALSE(op.isRepetitiveRegion(0));
+ EXPECT_FALSE(op.isRepetitiveRegion(1));
+ EXPECT_FALSE(op.hasLoop());
}
-TEST(RegionBranchOpInterface, LoopAfterEntryRegion) {
- const char *ir = R"MLIR(
-"cftest.entry_regions_op"() (
- {"cftest.dummy_op"() : () -> ()},
- {"cftest.dummy_op"() : () -> ()}
- ) {loop} : () -> ()
- )MLIR";
+TEST_F(RegionGraphTest, DiamondIsNotALoop) {
+ RegionBranchOpInterface op = createRegionGraphOp(
+ /*entries=*/{0}, /*successors=*/{{1, 2}, {3}, {3}, {}});
+ EXPECT_FALSE(op.hasLoop());
+}
- DialectRegistry registry;
- registry.insert<CFTestDialect>();
- MLIRContext ctx(registry);
+TEST_F(RegionGraphTest, LoopAfterEntryRegion) {
+ RegionBranchOpInterface op =
+ createRegionGraphOp(/*entries=*/{0}, /*successors=*/{{1}, {1}});
+ EXPECT_FALSE(op.isRepetitiveRegion(0));
+ EXPECT_TRUE(op.isRepetitiveRegion(1));
+ EXPECT_TRUE(op.hasLoop());
+}
- OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(ir, &ctx);
- ASSERT_TRUE(module);
- auto regionOp = cast<RegionBranchOpInterface>(
- &module->getBody()->getOperations().front());
+TEST_F(RegionGraphTest, LoopThroughEntryRegion) {
+ RegionBranchOpInterface op =
+ createRegionGraphOp(/*entries=*/{0}, /*successors=*/{{1}, {0}});
+ EXPECT_TRUE(op.hasLoop());
+}
- EXPECT_FALSE(regionOp.isRepetitiveRegion(0));
- EXPECT_TRUE(regionOp.isRepetitiveRegion(1));
- EXPECT_TRUE(regionOp.hasLoop());
+TEST_F(RegionGraphTest, UnreachableLoopIsIgnored) {
+ RegionBranchOpInterface op =
+ createRegionGraphOp(/*entries=*/{0}, /*successors=*/{{1}, {}, {2}});
+ EXPECT_TRUE(op.isRepetitiveRegion(2));
+ EXPECT_FALSE(op.hasLoop());
+}
+
+TEST_F(RegionGraphTest, OverlappingEntriesAreNotALoop) {
+ // Region 1 is an entry region and is also reachable from entry region 0.
+ // The search must not depend on the order of the entry regions.
+ EXPECT_FALSE(createRegionGraphOp(/*entries=*/{0, 1}, /*successors=*/{{1}, {}})
+ .hasLoop());
+ EXPECT_FALSE(createRegionGraphOp(/*entries=*/{1, 0}, /*successors=*/{{1}, {}})
+ .hasLoop());
}
More information about the Mlir-commits
mailing list