https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/220512
>From 019bc28b731f255e578ba73e17ba8c42f13c7c10 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 Six of the ten CIR ops implementing RegionBranchOpInterface reported the same successors: any of their regions may be entered from the parent operation, and every region exit goes back to it. Add a CIR_EnterAnyRegionBranchOpBase class that appends that definition to the one inherited from CIR_RegionBranchOpBase, retarget the six ops onto it and delete their hand-written definitions. The shared definition walks getRegions() rather than naming region accessors. For all six ops the entry regions were exactly the declared regions in declaration order, so it reports the same successors in the same order. The doc comments of the deleted ScopeOp and TernaryOp definitions go with them, instead of being left behind on the neighbouring builders. IfOp, GlobalOp, TryOp and AwaitOp stay on the base class, since their successors depend on the operation: IfOp falls back to the parent when the else region is empty, GlobalOp skips its optional ctor and dtor regions, TryOp iterates variadic handler regions, and AwaitOp routes ready to resume and suspend. --- clang/include/clang/CIR/Dialect/IR/CIROps.td | 33 +++++++-- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 77 -------------------- 2 files changed, 27 insertions(+), 83 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 9ab3c85cc7d6d..73e672cdd14a0 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -139,6 +139,27 @@ 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) { + // Every region branches back to the parent operation on exit. + if (!point.isParent()) { + regions.emplace_back(getOperation()); + return; + } + + for (mlir::Region ®ion : getOperation()->getRegions()) + regions.emplace_back(®ion); + } + }]; +} + //===----------------------------------------------------------------------===// // CIR Operation Traits //===----------------------------------------------------------------------===// @@ -1301,7 +1322,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 ]> { @@ -1398,7 +1419,7 @@ def CIR_CleanupKindAttr : CIR_EnumAttr<CIR_CleanupKind, "cleanup"> { }]; } -def CIR_CleanupScopeOp : CIR_RegionBranchOpBase<"cleanup.scope", [ +def CIR_CleanupScopeOp : CIR_EnterAnyRegionBranchOpBase<"cleanup.scope", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects ]> { @@ -1543,7 +1564,7 @@ def CIR_CaseOpKind : CIR_I32EnumAttr<"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"; @@ -1580,7 +1601,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 @@ -3251,7 +3272,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"; @@ -4838,7 +4859,7 @@ def CIR_AwaitOp : CIR_RegionBranchOpBase<"await", [ //===----------------------------------------------------------------------===// // CoroBody //===----------------------------------------------------------------------===// -def CIR_CoroBodyOp : CIR_RegionBranchOpBase<"coro.body", [ +def CIR_CoroBodyOp : CIR_EnterAnyRegionBranchOpBase<"coro.body", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects ]> { diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 847e0488a4ba5..223598f1bdbd9 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -1597,23 +1597,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) { @@ -1680,18 +1663,6 @@ LogicalResult cir::ScopeOp::fold(FoldAdaptor /*adaptor*/, // CleanupScopeOp //===----------------------------------------------------------------------===// -void cir::CleanupScopeOp::getSuccessorRegions( - mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) { - if (!point.isParent()) { - regions.emplace_back(getOperation()); - return; - } - - // Execution always proceeds from the body region to the cleanup region. - regions.push_back(RegionSuccessor(&getBodyRegion())); - regions.push_back(RegionSuccessor(&getCleanupRegion())); -} - LogicalResult cir::CleanupScopeOp::canonicalize(CleanupScopeOp op, PatternRewriter &rewriter) { auto isRegionTrivial = [](Region ®ion) { @@ -1874,15 +1845,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) { @@ -1900,16 +1862,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"); @@ -2933,25 +2885,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, @@ -3265,16 +3198,6 @@ LogicalResult cir::AwaitOp::verify() { // CoroBody //===----------------------------------------------------------------------===// -void cir::CoroBodyOp::getSuccessorRegions( - mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) { - if (!point.isParent()) { - regions.emplace_back(getOperation()); - return; - } - - regions.push_back(RegionSuccessor(&getBody())); -} - LogicalResult cir::CoroBodyOp::verify() { if (!getOperation()->getParentOfType<FuncOp>().getCoroutine()) return emitOpError("enclosing function must be a coroutine"); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
