llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangir
Author: Henrich Lauko (xlauko)
<details>
<summary>Changes</summary>
Adds unit tests for the five CIR ops that implement `RegionBranchOpInterface`
with no coverage today, and fixes a `cir.await` bug the new tests found:
`verifyControlFlowInterfaceConsistency` fails on `cir.await` today, because the
op and its own terminator disagree. `cir.condition` terminates the ready region
and reports `{resume, suspend}` when the parent is an await, while
`AwaitOp::getSuccessorRegions` listed all three regions as entry successors and
reported the parent op as the successor of every region exit.
The corrected edges match what `cir.condition` already reports:
- from the parent, the only entry successor is `ready`
- from `ready`, the successors are `resume` and `suspend`
- from `suspend` or `resume`, the successor is the parent operation
---
Full diff: https://github.com/llvm/llvm-project/pull/220507.diff
2 Files Affected:
- (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+20-7)
- (modified) clang/unittests/CIR/ControlFlowTest.cpp (+155)
``````````diff
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 4b1909c6f3fad..78995503191dc 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -3262,19 +3262,32 @@ void cir::AwaitOp::build(OpBuilder &builder,
OperationState &result,
void cir::AwaitOp::getSuccessorRegions(
mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) {
- // If any index all the underlying regions branch back to the parent
- // operation.
- if (!point.isParent()) {
- regions.emplace_back(getOperation());
+ assert(point.isParent() || point.getTerminatorPredecessorOrNull());
+
+ // Execution always starts in the ready region.
+ if (point.isParent()) {
+ regions.emplace_back(&getReady());
return;
}
+ mlir::Region *parentRegion =
+ point.getTerminatorPredecessorOrNull()->getParentRegion();
+
+ // Branching from ready: the cir.condition terminating it selects between
+ // suspending and resuming. Keep in sync with
+ // ConditionOp::getSuccessorRegions.
+ //
// TODO: retrieve information from the promise and only push the
// necessary ones. Example: `std::suspend_never` on initial or final
// await's might allow suspend region to be skipped.
- regions.push_back(RegionSuccessor(&this->getReady()));
- regions.push_back(RegionSuccessor(&this->getSuspend()));
- regions.push_back(RegionSuccessor(&this->getResume()));
+ if (&getReady() == parentRegion) {
+ regions.emplace_back(&getResume());
+ regions.emplace_back(&getSuspend());
+ return;
+ }
+
+ // Branching from suspend or resume: exit to the parent operation.
+ regions.emplace_back(getOperation());
}
mlir::ValueRange cir::AwaitOp::getSuccessorInputs(RegionSuccessor successor) {
diff --git a/clang/unittests/CIR/ControlFlowTest.cpp
b/clang/unittests/CIR/ControlFlowTest.cpp
index 07df0bb839364..d6b9fda5235e6 100644
--- a/clang/unittests/CIR/ControlFlowTest.cpp
+++ b/clang/unittests/CIR/ControlFlowTest.cpp
@@ -320,6 +320,32 @@ TEST_F(CIRControlFlowTest, SwitchOp) {
verifyControlFlowInterfaceConsistency(switchOp);
}
+TEST_F(CIRControlFlowTest, CaseOp) {
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ !s32i = !cir.int<s, 32>
+ cir.func @f(%val : !s32i) {
+ cir.switch (%val : !s32i) {
+ cir.case (equal, [#cir.int<1> : !s32i]) {
+ cir.yield
+ }
+ cir.yield
+ }
+ cir.return
+ }
+ )CIR");
+ auto caseOp = findFirstOp<cir::CaseOp>(*module);
+
+ expectSuccessors(caseOp, RegionBranchPoint::parent(),
+ {&caseOp.getCaseRegion()});
+ expectTerminatorSuccessors(caseOp.getCaseRegion(), {nullptr});
+
+ RegionBranchOpInterface caseBranch = asRegionBranch(caseOp);
+ EXPECT_FALSE(caseBranch.isRepetitiveRegion(0));
+ EXPECT_FALSE(caseBranch.hasLoop());
+
+ verifyControlFlowInterfaceConsistency(caseOp);
+}
+
TEST_F(CIRControlFlowTest, WhileOp) {
OwningOpRef<ModuleOp> module = parse(R"CIR(
cir.func @f(%cond : !cir.bool) {
@@ -487,6 +513,67 @@ TEST_F(CIRControlFlowTest, ForOpWithCleanup) {
verifyControlFlowInterfaceConsistency(forOp);
}
+TEST_F(CIRControlFlowTest, CleanupScopeOp) {
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ cir.func @f() {
+ cir.cleanup.scope {
+ cir.yield
+ } cleanup all {
+ cir.yield
+ }
+ cir.return
+ }
+ )CIR");
+ auto cleanupScopeOp = findFirstOp<cir::CleanupScopeOp>(*module);
+
+ expectSuccessors(
+ cleanupScopeOp, RegionBranchPoint::parent(),
+ {&cleanupScopeOp.getBodyRegion(), &cleanupScopeOp.getCleanupRegion()});
+ expectTerminatorSuccessors(cleanupScopeOp.getBodyRegion(), {nullptr});
+ expectTerminatorSuccessors(cleanupScopeOp.getCleanupRegion(), {nullptr});
+
+ RegionBranchOpInterface cleanupBranch = asRegionBranch(cleanupScopeOp);
+ EXPECT_FALSE(cleanupBranch.isRepetitiveRegion(0));
+ EXPECT_FALSE(cleanupBranch.isRepetitiveRegion(1));
+ EXPECT_FALSE(cleanupBranch.hasLoop());
+
+ verifyControlFlowInterfaceConsistency(cleanupScopeOp);
+}
+
+TEST_F(CIRControlFlowTest, GlobalOpWithCtorAndDtor) {
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ !s32i = !cir.int<s, 32>
+ cir.global external @g = ctor : !s32i {
+ cir.yield
+ } dtor {
+ cir.yield
+ }
+ )CIR");
+ auto globalOp = findFirstOp<cir::GlobalOp>(*module);
+
+ expectSuccessors(globalOp, RegionBranchPoint::parent(),
+ {&globalOp.getCtorRegion(), &globalOp.getDtorRegion()});
+ expectTerminatorSuccessors(globalOp.getCtorRegion(), {nullptr});
+ expectTerminatorSuccessors(globalOp.getDtorRegion(), {nullptr});
+
+ EXPECT_FALSE(asRegionBranch(globalOp).hasLoop());
+
+ verifyControlFlowInterfaceConsistency(globalOp);
+}
+
+TEST_F(CIRControlFlowTest, GlobalOpWithoutRegions) {
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ !s32i = !cir.int<s, 32>
+ cir.global external @g = #cir.int<0> : !s32i
+ )CIR");
+ auto globalOp = findFirstOp<cir::GlobalOp>(*module);
+
+ // A global with neither a ctor nor a dtor never enters a region, so it has
+ // no successors at all. verifyControlFlowInterfaceConsistency doesn't apply:
+ // it requires the parent to be reachable from some branch point.
+ expectSuccessors(globalOp, RegionBranchPoint::parent(), {});
+}
+
TEST_F(CIRControlFlowTest, TryOpWithCatchAll) {
OwningOpRef<ModuleOp> module = parse(R"CIR(
!void = !cir.void
@@ -525,3 +612,71 @@ TEST_F(CIRControlFlowTest, TryOpWithCatchAll) {
// TODO: TryOp::getSuccessorInputs returns empty for handler regions that
// have block arguments, so verifyControlFlowInterfaceConsistency fails.
}
+
+TEST_F(CIRControlFlowTest, CoroBodyOp) {
+ // A coroutine body must contain at least one cir.await.
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ cir.func coroutine @f(%arg0 : !cir.bool) {
+ cir.coro.body {
+ cir.await(user, ready : {
+ cir.condition(%arg0)
+ }, suspend : {
+ cir.yield
+ }, resume : {
+ cir.yield
+ },)
+ cir.yield
+ }
+ cir.return
+ }
+ )CIR");
+ auto coroBodyOp = findFirstOp<cir::CoroBodyOp>(*module);
+
+ expectSuccessors(coroBodyOp, RegionBranchPoint::parent(),
+ {&coroBodyOp.getBody()});
+ expectTerminatorSuccessors(coroBodyOp.getBody(), {nullptr});
+
+ RegionBranchOpInterface coroBranch = asRegionBranch(coroBodyOp);
+ EXPECT_FALSE(coroBranch.isRepetitiveRegion(0));
+ EXPECT_FALSE(coroBranch.hasLoop());
+
+ verifyControlFlowInterfaceConsistency(coroBodyOp);
+}
+
+TEST_F(CIRControlFlowTest, AwaitOp) {
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ cir.func coroutine @f(%arg0 : !cir.bool) {
+ cir.coro.body {
+ cir.await(user, ready : {
+ cir.condition(%arg0)
+ }, suspend : {
+ cir.yield
+ }, resume : {
+ cir.yield
+ },)
+ cir.yield
+ }
+ cir.return
+ }
+ )CIR");
+ auto awaitOp = findFirstOp<cir::AwaitOp>(*module);
+
+ // Only the ready region is entered from the parent; suspend and resume are
+ // selected by the cir.condition terminating it.
+ expectSuccessors(awaitOp, RegionBranchPoint::parent(),
{&awaitOp.getReady()});
+
+ RegionBranchTerminatorOpInterface readyTerm =
+ getTerminator(awaitOp.getReady());
+ ASSERT_TRUE(readyTerm);
+ expectSuccessors(awaitOp, RegionBranchPoint(readyTerm),
+ {&awaitOp.getResume(), &awaitOp.getSuspend()});
+ expectTerminatorSuccessors(awaitOp.getReady(),
+ {&awaitOp.getResume(), &awaitOp.getSuspend()});
+
+ expectTerminatorSuccessors(awaitOp.getSuspend(), {nullptr});
+ expectTerminatorSuccessors(awaitOp.getResume(), {nullptr});
+
+ EXPECT_FALSE(asRegionBranch(awaitOp).hasLoop());
+
+ verifyControlFlowInterfaceConsistency(awaitOp);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/220507
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
