From 6c204fdade935594a689ca3003fe7e9f8ba164f3 Mon Sep 17 00:00:00 2001 From: Amr Hesham Date: Tue, 28 Oct 2025 18:16:28 +0100 Subject: [PATCH 1/2] [CIR] Fix building ClangIR after RegionBranchOpInterface revamp --- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 23 +++++++++++-------- .../lib/CIR/Interfaces/CIRLoopOpInterface.cpp | 13 +++++++---- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 2d2ef422bfaef..7ba03ce40140c 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -286,14 +286,14 @@ void cir::ConditionOp::getSuccessorRegions( // Parent is a loop: condition may branch to the body or to the parent op. if (auto loopOp = dyn_cast(getOperation()->getParentOp())) { regions.emplace_back(&loopOp.getBody(), loopOp.getBody().getArguments()); - regions.emplace_back(loopOp->getResults()); + regions.emplace_back(getOperation(), loopOp->getResults()); } assert(!cir::MissingFeatures::awaitOp()); } MutableOperandRange -cir::ConditionOp::getMutableSuccessorOperands(RegionBranchPoint point) { +cir::ConditionOp::getMutableSuccessorOperands(RegionSuccessor point) { // No values are yielded to the successor region. return MutableOperandRange(getOperation(), 0, 0); } @@ -989,7 +989,8 @@ void cir::IfOp::getSuccessorRegions(mlir::RegionBranchPoint point, SmallVectorImpl ®ions) { // The `then` and the `else` region branch back to the parent operation. if (!point.isParent()) { - regions.push_back(RegionSuccessor()); + regions.push_back( + RegionSuccessor(getOperation(), getOperation()->getResults())); return; } @@ -1039,7 +1040,7 @@ void cir::ScopeOp::getSuccessorRegions( mlir::RegionBranchPoint point, SmallVectorImpl ®ions) { // The only region always branch back to the parent operation. if (!point.isParent()) { - regions.push_back(RegionSuccessor(getODSResults(0))); + regions.push_back(RegionSuccessor(getOperation(), getODSResults(0))); return; } @@ -1124,7 +1125,8 @@ Block *cir::BrCondOp::getSuccessorForOperands(ArrayRef operands) { void cir::CaseOp::getSuccessorRegions( mlir::RegionBranchPoint point, SmallVectorImpl ®ions) { if (!point.isParent()) { - regions.push_back(RegionSuccessor()); + regions.push_back( + RegionSuccessor(getOperation(), getOperation()->getResults())); return; } regions.push_back(RegionSuccessor(&getCaseRegion())); @@ -1188,7 +1190,8 @@ static void printSwitchOp(OpAsmPrinter &p, cir::SwitchOp op, void cir::SwitchOp::getSuccessorRegions( mlir::RegionBranchPoint point, SmallVectorImpl ®ion) { if (!point.isParent()) { - region.push_back(RegionSuccessor()); + region.push_back( + RegionSuccessor(getOperation(), getOperation()->getResults())); return; } @@ -1402,7 +1405,8 @@ void cir::GlobalOp::getSuccessorRegions( mlir::RegionBranchPoint point, SmallVectorImpl ®ions) { // The `ctor` and `dtor` regions always branch back to the parent operation. if (!point.isParent()) { - regions.push_back(RegionSuccessor()); + regions.push_back( + RegionSuccessor(getOperation(), getOperation()->getResults())); return; } @@ -1961,7 +1965,7 @@ void cir::TernaryOp::getSuccessorRegions( mlir::RegionBranchPoint point, SmallVectorImpl ®ions) { // The `true` and the `false` region branch back to the parent operation. if (!point.isParent()) { - regions.push_back(RegionSuccessor(this->getODSResults(0))); + regions.push_back(RegionSuccessor(getOperation(), this->getODSResults(0))); return; } @@ -2978,7 +2982,8 @@ void cir::TryOp::getSuccessorRegions( llvm::SmallVectorImpl ®ions) { // The `try` and the `catchers` region branch back to the parent operation. if (!point.isParent()) { - regions.push_back(mlir::RegionSuccessor()); + regions.push_back( + RegionSuccessor(getOperation(), getOperation()->getResults())); return; } diff --git a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp b/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp index 0ce5017a399da..0fbfbea828db4 100644 --- a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp +++ b/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp @@ -17,7 +17,7 @@ namespace cir { void LoopOpInterface::getLoopOpSuccessorRegions( LoopOpInterface op, mlir::RegionBranchPoint point, llvm::SmallVectorImpl ®ions) { - assert(point.isParent() || point.getRegionOrNull()); + assert(point.isParent() || point.getTerminatorPredecessorOrNull()); // Branching to first region: go to condition or body (do-while). if (point.isParent()) { @@ -26,14 +26,16 @@ void LoopOpInterface::getLoopOpSuccessorRegions( } // Branching from condition: go to body or exit. - if (&op.getCond() == point.getRegionOrNull()) { - regions.emplace_back(mlir::RegionSuccessor(op->getResults())); + if (&op.getCond() == + point.getTerminatorPredecessorOrNull()->getParentRegion()) { + regions.emplace_back(mlir::RegionSuccessor(op, op->getResults())); regions.emplace_back(&op.getBody(), op.getBody().getArguments()); return; } // Branching from body: go to step (for) or condition. - if (&op.getBody() == point.getRegionOrNull()) { + if (&op.getBody() == + point.getTerminatorPredecessorOrNull()->getParentRegion()) { // FIXME(cir): Should we consider break/continue statements here? mlir::Region *afterBody = (op.maybeGetStep() ? op.maybeGetStep() : &op.getCond()); @@ -42,7 +44,8 @@ void LoopOpInterface::getLoopOpSuccessorRegions( } // Branching from step: go to condition. - if (op.maybeGetStep() == point.getRegionOrNull()) { + if (op.maybeGetStep() == + point.getTerminatorPredecessorOrNull()->getParentRegion()) { regions.emplace_back(&op.getCond(), op.getCond().getArguments()); return; } From f10a7eb80343ddd46b4b5884d2415adb9ab85753 Mon Sep 17 00:00:00 2001 From: Amr Hesham Date: Tue, 28 Oct 2025 19:03:31 +0100 Subject: [PATCH 2/2] Address code review comments --- clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp b/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp index 0fbfbea828db4..6de51f12837ba 100644 --- a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp +++ b/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp @@ -25,17 +25,18 @@ void LoopOpInterface::getLoopOpSuccessorRegions( return; } + mlir::Region *parentRegion = + point.getTerminatorPredecessorOrNull()->getParentRegion(); + // Branching from condition: go to body or exit. - if (&op.getCond() == - point.getTerminatorPredecessorOrNull()->getParentRegion()) { + if (&op.getCond() == parentRegion) { regions.emplace_back(mlir::RegionSuccessor(op, op->getResults())); regions.emplace_back(&op.getBody(), op.getBody().getArguments()); return; } // Branching from body: go to step (for) or condition. - if (&op.getBody() == - point.getTerminatorPredecessorOrNull()->getParentRegion()) { + if (&op.getBody() == parentRegion) { // FIXME(cir): Should we consider break/continue statements here? mlir::Region *afterBody = (op.maybeGetStep() ? op.maybeGetStep() : &op.getCond()); @@ -44,8 +45,7 @@ void LoopOpInterface::getLoopOpSuccessorRegions( } // Branching from step: go to condition. - if (op.maybeGetStep() == - point.getTerminatorPredecessorOrNull()->getParentRegion()) { + if (op.maybeGetStep() == parentRegion) { regions.emplace_back(&op.getCond(), op.getCond().getArguments()); return; }