[clang] fbcd45a - [CIR][NFC] Share getSuccessorRegions across region-branch ops (#220512)

via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 13 04:09:04 PDT 2026 

Author: Henrich Lauko
Date: 2026-10-13T13:08:57+02:00
New Revision: fbcd45affab077e6936036e3c836e07095fd0a3d

URL: https://github.com/llvm/llvm-project/commit/fbcd45affab077e6936036e3c836e07095fd0a3d
DIFF: https://github.com/llvm/llvm-project/commit/fbcd45affab077e6936036e3c836e07095fd0a3d.diff

LOG: [CIR][NFC] Share getSuccessorRegions across region-branch ops (#220512)

Four of the ten CIR ops that declare `RegionBranchOpInterface` directly
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 stale 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 the fixes can land separately, in
#221222 and #231449.

Assisted-by: Claude Code (Claude Opus 5.5).

Added: 
    

Modified: 
    clang/include/clang/CIR/Dialect/IR/CIROps.td
    clang/lib/CIR/Dialect/IR/CIRDialect.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index b0b3d03cefa59..59e67bddc3e14 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> &regions) {
+      if (!point.isParent()) {
+        regions.emplace_back(getOperation());
+        return;
+      }
+
+      for (mlir::Region &region : getOperation()->getRegions())
+        regions.emplace_back(&region);
+    }
+  }];
+}
+
 //===----------------------------------------------------------------------===//
 // 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
@@ -3296,7 +3316,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> &regions) {
-  // 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> &regions) {
-  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> &region) {
-  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> &regions) {
-  // 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,

More information about the cfe-commits mailing list