Author: Henrich Lauko Date: 2026-09-04T16:31:18+02:00 New Revision: ee5d07b1655fa0e735d8caed6a447204cb170be9
URL: https://github.com/llvm/llvm-project/commit/ee5d07b1655fa0e735d8caed6a447204cb170be9 DIFF: https://github.com/llvm/llvm-project/commit/ee5d07b1655fa0e735d8caed6a447204cb170be9.diff LOG: [CIR][NFC] Share getSuccessorInputs across region-branch ops (#220509) All ten CIR ops implementing `RegionBranchOpInterface` hand-wrote `getSuccessorInputs`, and all ten implementations expressed the same behavior: regions take no inputs, while returning to the parent yields the parent’s results. Three appeared different but were equivalent: - `CleanupScopeOp` and `CoroBodyOp` unconditionally returned an empty ValueRange, and neither declares results in ODS. - `AwaitOp` returned region block arguments, but it carries `NoRegionArguments`, so those ranges are always empty. `CIR_RegionBranchOpBase` now declares the method and generates the shared implementation through extraClassDefinition. 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 1a4afd3b53d8b..9ab3c85cc7d6d 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -121,6 +121,24 @@ class CIR_Op<string mnemonic, list<Trait> traits = []> : LoweringBuilders customLLVMLoweringConstructorDecl = ?; } +// Base class for structured control flow ops whose regions take no inputs and +// yield the parent's results on the way out. Users still have to define +// `getSuccessorRegions` themselves. +class CIR_RegionBranchOpBase<string mnemonic, list<Trait> traits = []> + : CIR_Op<mnemonic, !listconcat([ + DeclareOpInterfaceMethods<RegionBranchOpInterface, + ["getSuccessorInputs"]>], traits)> { + let extraClassDefinition = [{ + ValueRange $cppClass::getSuccessorInputs( + mlir::RegionSuccessor successor) { + // Regions take no inputs; returning to the parent yields its results. + return successor.isOperation() + ? ValueRange(getOperation()->getResults()) + : ValueRange(); + } + }]; +} + //===----------------------------------------------------------------------===// // CIR Operation Traits //===----------------------------------------------------------------------===// @@ -992,8 +1010,7 @@ def CIR_ReturnOp : CIR_Op<"return", [ // IfOp //===----------------------------------------------------------------------===// -def CIR_IfOp : CIR_Op<"if", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_IfOp : CIR_RegionBranchOpBase<"if", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments ]> { let summary = "the if-then-else operation"; @@ -1284,8 +1301,7 @@ def CIR_ResumeFlatOp : CIR_Op<"resume.flat", [ // ScopeOp //===----------------------------------------------------------------------===// -def CIR_ScopeOp : CIR_Op<"scope", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_ScopeOp : CIR_RegionBranchOpBase<"scope", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects ]> { @@ -1382,8 +1398,7 @@ def CIR_CleanupKindAttr : CIR_EnumAttr<CIR_CleanupKind, "cleanup"> { }]; } -def CIR_CleanupScopeOp : CIR_Op<"cleanup.scope", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_CleanupScopeOp : CIR_RegionBranchOpBase<"cleanup.scope", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects ]> { @@ -1528,8 +1543,7 @@ def CIR_CaseOpKind : CIR_I32EnumAttr<"CaseOpKind", "case kind", [ def CIR_CaseOpKindAttr : CIR_EnumAttr<CIR_CaseOpKind, "case">; -def CIR_CaseOp : CIR_Op<"case", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_CaseOp : CIR_RegionBranchOpBase<"case", [ RecursivelySpeculatable, AutomaticAllocationScope ]> { let summary = "Case operation"; @@ -1566,9 +1580,8 @@ def CIR_CaseOp : CIR_Op<"case", [ let hasLLVMLowering = false; } -def CIR_SwitchOp : CIR_Op<"switch", [ +def CIR_SwitchOp : CIR_RegionBranchOpBase<"switch", [ SameVariadicOperandSize, - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects ]> { @@ -3238,8 +3251,7 @@ def CIR_SelectOp : CIR_Op<"select", [ // TernaryOp //===----------------------------------------------------------------------===// -def CIR_TernaryOp : CIR_Op<"ternary", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_TernaryOp : CIR_RegionBranchOpBase<"ternary", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments ]> { let summary = "The `cond ? a : b` C/C++ ternary operation"; @@ -3348,8 +3360,7 @@ def CIR_TLSModelAttr: CIR_EnumAttr<CIR_TLSModel, "tls_model"> { }]; } -def CIR_GlobalOp : CIR_Op<"global", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_GlobalOp : CIR_RegionBranchOpBase<"global", [ SymbolName, SymbolVisibility, DeclareOpInterfaceMethods<CIRGlobalValueInterface>, NoRegionArguments @@ -4742,8 +4753,7 @@ def CIR_AwaitKind : CIR_I32EnumAttr<"AwaitKind", "await kind", [ def CIR_AwaitKindAttr : CIR_EnumAttr<CIR_AwaitKind, "await">; -def CIR_AwaitOp : CIR_Op<"await",[ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_AwaitOp : CIR_RegionBranchOpBase<"await", [ RecursivelySpeculatable, NoRegionArguments ]> { let summary = "Wraps C++ co_await implicit logic"; @@ -4828,8 +4838,7 @@ def CIR_AwaitOp : CIR_Op<"await",[ //===----------------------------------------------------------------------===// // CoroBody //===----------------------------------------------------------------------===// -def CIR_CoroBodyOp : CIR_Op<"coro.body", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_CoroBodyOp : CIR_RegionBranchOpBase<"coro.body", [ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments, RecursiveMemoryEffects ]> { @@ -8153,8 +8162,7 @@ def CIR_AllocExceptionOp : CIR_Op<"alloc.exception"> { // TryOp //===----------------------------------------------------------------------===// -def CIR_TryOp : CIR_Op<"try",[ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_TryOp : CIR_RegionBranchOpBase<"try", [ RecursivelySpeculatable, AutomaticAllocationScope ]> { let summary = "C++ try block"; diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index c3838413984a6..847e0488a4ba5 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -1574,11 +1574,6 @@ void cir::IfOp::getSuccessorRegions(mlir::RegionBranchPoint point, regions.emplace_back(getOperation()); } -mlir::ValueRange cir::IfOp::getSuccessorInputs(RegionSuccessor successor) { - return successor.isOperation() ? ValueRange(getOperation()->getResults()) - : ValueRange(); -} - void cir::IfOp::build(OpBuilder &builder, OperationState &result, Value cond, bool withElseRegion, BuilderCallbackRef thenBuilder, BuilderCallbackRef elseBuilder) { @@ -1619,11 +1614,6 @@ void cir::ScopeOp::getSuccessorRegions( regions.push_back(RegionSuccessor(&getScopeRegion())); } -mlir::ValueRange cir::ScopeOp::getSuccessorInputs(RegionSuccessor successor) { - return successor.isOperation() ? ValueRange(getOperation()->getResults()) - : ValueRange(); -} - void cir::ScopeOp::build( OpBuilder &builder, OperationState &result, function_ref<void(OpBuilder &, Type &, Location)> scopeBuilder) { @@ -1702,11 +1692,6 @@ void cir::CleanupScopeOp::getSuccessorRegions( regions.push_back(RegionSuccessor(&getCleanupRegion())); } -mlir::ValueRange -cir::CleanupScopeOp::getSuccessorInputs(RegionSuccessor successor) { - return ValueRange(); -} - LogicalResult cir::CleanupScopeOp::canonicalize(CleanupScopeOp op, PatternRewriter &rewriter) { auto isRegionTrivial = [](Region ®ion) { @@ -1898,11 +1883,6 @@ void cir::CaseOp::getSuccessorRegions( regions.push_back(RegionSuccessor(&getCaseRegion())); } -mlir::ValueRange cir::CaseOp::getSuccessorInputs(RegionSuccessor successor) { - return successor.isOperation() ? ValueRange(getOperation()->getResults()) - : ValueRange(); -} - void cir::CaseOp::build(OpBuilder &builder, OperationState &result, ArrayAttr value, CaseOpKind kind, OpBuilder::InsertPoint &insertPoint) { @@ -1930,11 +1910,6 @@ void cir::SwitchOp::getSuccessorRegions( region.push_back(RegionSuccessor(&getBody())); } -mlir::ValueRange cir::SwitchOp::getSuccessorInputs(RegionSuccessor successor) { - return successor.isOperation() ? ValueRange(getOperation()->getResults()) - : ValueRange(); -} - void cir::SwitchOp::build(OpBuilder &builder, OperationState &result, Value cond, BuilderOpStateCallbackRef switchBuilder) { assert(switchBuilder && "the builder callback for regions must be present"); @@ -2190,11 +2165,6 @@ void cir::GlobalOp::getSuccessorRegions( regions.push_back(RegionSuccessor(dtorRegion)); } -mlir::ValueRange cir::GlobalOp::getSuccessorInputs(RegionSuccessor successor) { - return successor.isOperation() ? ValueRange(getOperation()->getResults()) - : ValueRange(); -} - static void printGlobalOpTypeAndInitialValue(OpAsmPrinter &p, cir::GlobalOp op, TypeAttr type, Attribute initAttr, mlir::Region &ctorRegion, @@ -2982,11 +2952,6 @@ void cir::TernaryOp::getSuccessorRegions( regions.push_back(RegionSuccessor(&getFalseRegion())); } -mlir::ValueRange cir::TernaryOp::getSuccessorInputs(RegionSuccessor successor) { - return successor.isOperation() ? ValueRange(getOperation()->getResults()) - : ValueRange(); -} - void cir::TernaryOp::build( OpBuilder &builder, OperationState &result, Value cond, function_ref<void(OpBuilder &, Location)> trueBuilder, @@ -3290,18 +3255,6 @@ void cir::AwaitOp::getSuccessorRegions( regions.emplace_back(getOperation()); } -mlir::ValueRange cir::AwaitOp::getSuccessorInputs(RegionSuccessor successor) { - if (successor.isOperation()) - return getOperation()->getResults(); - if (successor == &getReady()) - return getReady().getArguments(); - if (successor == &getSuspend()) - return getSuspend().getArguments(); - if (successor == &getResume()) - return getResume().getArguments(); - llvm_unreachable("invalid region successor"); -} - LogicalResult cir::AwaitOp::verify() { if (!isa<ConditionOp>(this->getReady().back().getTerminator())) return emitOpError("ready region must end with cir.condition"); @@ -3322,11 +3275,6 @@ void cir::CoroBodyOp::getSuccessorRegions( regions.push_back(RegionSuccessor(&getBody())); } -mlir::ValueRange -cir::CoroBodyOp::getSuccessorInputs(RegionSuccessor successor) { - return ValueRange(); -} - LogicalResult cir::CoroBodyOp::verify() { if (!getOperation()->getParentOfType<FuncOp>().getCoroutine()) return emitOpError("enclosing function must be a coroutine"); @@ -4431,11 +4379,6 @@ void cir::TryOp::getSuccessorRegions( regions.push_back(mlir::RegionSuccessor(&handlerRegion)); } -mlir::ValueRange cir::TryOp::getSuccessorInputs(RegionSuccessor successor) { - return successor.isOperation() ? ValueRange(getOperation()->getResults()) - : ValueRange(); -} - LogicalResult cir::TryOp::verify() { mlir::ArrayAttr handlerTypes = getHandlerTypes(); if (!handlerTypes) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
