https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/220507
>From e0841e3fed8e043f5eb8d3e266b977be6f0a09a9 Mon Sep 17 00:00:00 2001 From: Henrich Lauko <[email protected]> Date: Wed, 2 Sep 2026 07:56:29 +0000 Subject: [PATCH] [CIR] Add RegionBranchOpInterface unit tests and fix cir.await successors Five of the ten ops implementing RegionBranchOpInterface have no unit test coverage: cir.case, cir.cleanup.scope, cir.global, cir.await and cir.coro.body. Add tests for all five. Covering cir.await exposes a disagreement with its own terminator. cir.condition terminates the ready region and reports {resume, suspend} as its successors when the parent is an await, but AwaitOp::getSuccessorRegions listed all three regions as entry successors and reported the parent op as the successor of every region exit. Fix it to match cir.condition: ready is the only entry successor, exiting ready branches to resume or suspend, and exiting suspend or resume returns to the parent operation. cir.await declares no results and carries NoRegionArguments, so successor operand and input counts stay at zero along every edge and the MLIR verifier is unaffected. --- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 27 +++-- clang/unittests/CIR/ControlFlowTest.cpp | 155 ++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 7 deletions(-) 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); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
