https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/220512
>From bbebad5fa225bf840abcae9f6dde25fbb07d9b21 Mon Sep 17 00:00:00 2001 From: Henrich Lauko <[email protected]> Date: Wed, 2 Sep 2026 08:37:42 +0000 Subject: [PATCH] [CIR][NFC] Share getSuccessorRegions across region-branch ops Four of the ten CIR ops implementing `RegionBranchOpInterface` had identical `getSuccessorRegions` bodies. The parent may enter any of their regions, and every region exits back to the parent. This moves that body into a new TableGen base class, `CIR_EnterAnyRegionBranchOpBase`, and puts `ScopeOp`, `CaseOp`, `SwitchOp` and `TernaryOp` on it. The doc comments above the deleted `ScopeOp` and `TernaryOp` definitions go too. `IfOp`, `GlobalOp`, `AwaitOp` and `CoroutineOp` keep their own definitions because their successors differ. `IfOp` exits straight to the parent when the else region is empty, `GlobalOp` skips empty ctor and dtor regions, and `AwaitOp` and `CoroutineOp` send each region to specific successors. `CleanupScopeOp` and `TryOp` have the same successors today, but both are wrong. The cleanup region runs after the body, and a catch handler runs after the try body unwinds, so neither is an entry from the parent. They keep their own definitions so each fix can land on its own. PR #221222 fixes `CleanupScopeOp`. --- clang/include/clang/CIR/Dialect/IR/CIROps.td | 28 ++++++++-- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 55 -------------------- 2 files changed, 24 insertions(+), 59 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index babd9ed5a3d92..9308ec7d0e7d8 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -139,6 +139,26 @@ class CIR_RegionBranchOpBase<string mnemonic, list<Trait> traits = []> }]; } +// Base class for the region-branch ops whose successors do not depend on the +// operation: any of their regions may be entered from the parent operation, and +// every region returns to it on exit. +class CIR_EnterAnyRegionBranchOpBase<string mnemonic, list<Trait> traits = []> + : CIR_RegionBranchOpBase<mnemonic, traits> { + let append extraClassDefinition = [{ + void $cppClass::getSuccessorRegions( + mlir::RegionBranchPoint point, + llvm::SmallVectorImpl<mlir::RegionSuccessor> ®ions) { + if (!point.isParent()) { + regions.emplace_back(getOperation()); + return; + } + + for (mlir::Region ®ion : getOperation()->getRegions()) + regions.emplace_back(®ion); + } + }]; +} + //===----------------------------------------------------------------------===// // CIR Operation Traits //===----------------------------------------------------------------------===// @@ -1351,7 +1371,7 @@ def CIR_ResumeFlatOp : CIR_Op<"resume.flat", [ // ScopeOp //===----------------------------------------------------------------------===// -def CIR_ScopeOp : CIR_RegionBranchOpBase<"scope", [ +def CIR_ScopeOp : CIR_EnterAnyRegionBranchOpBase<"scope", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects ]> { @@ -1575,7 +1595,7 @@ def CIR_CaseOpKind : CIR_I32Enum<"CaseOpKind", "case kind", [ def CIR_CaseOpKindAttr : CIR_EnumAttr<CIR_CaseOpKind, "case">; -def CIR_CaseOp : CIR_RegionBranchOpBase<"case", [ +def CIR_CaseOp : CIR_EnterAnyRegionBranchOpBase<"case", [ RecursivelySpeculatable, AutomaticAllocationScope ]> { let summary = "Case operation"; @@ -1612,7 +1632,7 @@ def CIR_CaseOp : CIR_RegionBranchOpBase<"case", [ let hasLLVMLowering = false; } -def CIR_SwitchOp : CIR_RegionBranchOpBase<"switch", [ +def CIR_SwitchOp : CIR_EnterAnyRegionBranchOpBase<"switch", [ SameVariadicOperandSize, RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects @@ -3290,7 +3310,7 @@ def CIR_SelectOp : CIR_Op<"select", [ // TernaryOp //===----------------------------------------------------------------------===// -def CIR_TernaryOp : CIR_RegionBranchOpBase<"ternary", [ +def CIR_TernaryOp : CIR_EnterAnyRegionBranchOpBase<"ternary", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments ]> { let summary = "The `cond ? a : b` C/C++ ternary operation"; diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 0fb9f585d2228..f02288dd92f69 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -1912,23 +1912,6 @@ void cir::IfOp::build(OpBuilder &builder, OperationState &result, Value cond, // ScopeOp //===----------------------------------------------------------------------===// -/// Given the region at `index`, or the parent operation if `index` is None, -/// return the successor regions. These are the regions that may be selected -/// during the flow of control. `operands` is a set of optional attributes -/// that correspond to a constant value for each operand, or null if that -/// operand is not a constant. -void cir::ScopeOp::getSuccessorRegions( - mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) { - // The only region always branch back to the parent operation. - if (!point.isParent()) { - regions.emplace_back(getOperation()); - return; - } - - // If the condition isn't constant, both regions may be executed. - regions.push_back(RegionSuccessor(&getScopeRegion())); -} - void cir::ScopeOp::build( OpBuilder &builder, OperationState &result, function_ref<void(OpBuilder &, Type &, Location)> scopeBuilder) { @@ -2239,15 +2222,6 @@ Block *cir::BrCondOp::getSuccessorForOperands(ArrayRef<Attribute> operands) { // CaseOp //===----------------------------------------------------------------------===// -void cir::CaseOp::getSuccessorRegions( - mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) { - if (!point.isParent()) { - regions.emplace_back(getOperation()); - return; - } - regions.push_back(RegionSuccessor(&getCaseRegion())); -} - void cir::CaseOp::build(OpBuilder &builder, OperationState &result, ArrayAttr value, CaseOpKind kind, OpBuilder::InsertPoint &insertPoint) { @@ -2265,16 +2239,6 @@ void cir::CaseOp::build(OpBuilder &builder, OperationState &result, // SwitchOp //===----------------------------------------------------------------------===// -void cir::SwitchOp::getSuccessorRegions( - mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ion) { - if (!point.isParent()) { - region.emplace_back(getOperation()); - return; - } - - region.push_back(RegionSuccessor(&getBody())); -} - void cir::SwitchOp::build(OpBuilder &builder, OperationState &result, Value cond, BuilderOpStateCallbackRef switchBuilder) { assert(switchBuilder && "the builder callback for regions must be present"); @@ -3404,25 +3368,6 @@ LogicalResult cir::SubOp::verify() { // TernaryOp //===----------------------------------------------------------------------===// -/// Given the region at `point`, or the parent operation if `point` is None, -/// return the successor regions. These are the regions that may be selected -/// during the flow of control. `operands` is a set of optional attributes that -/// correspond to a constant value for each operand, or null if that operand is -/// not a constant. -void cir::TernaryOp::getSuccessorRegions( - mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) { - // The `true` and the `false` region branch back to the parent operation. - if (!point.isParent()) { - regions.emplace_back(getOperation()); - return; - } - - // When branching from the parent operation, both the true and false - // regions are considered possible successors - regions.push_back(RegionSuccessor(&getTrueRegion())); - regions.push_back(RegionSuccessor(&getFalseRegion())); -} - void cir::TernaryOp::build( OpBuilder &builder, OperationState &result, Value cond, function_ref<void(OpBuilder &, Location)> trueBuilder, _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
