https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/220509
>From 0e8e9cc913bfbf1d2f00653a5fcd3e6846e46cf1 Mon Sep 17 00:00:00 2001 From: Henrich Lauko <[email protected]> Date: Wed, 2 Sep 2026 08:09:26 +0000 Subject: [PATCH] [CIR][NFC] Share getSuccessorInputs across region-branch ops The ten CIR ops implementing RegionBranchOpInterface each hand-wrote getSuccessorInputs, and all ten bodies were equivalent: regions take no inputs, and returning to the parent yields the parent's results. Three did not look equivalent but are: CleanupScopeOp and CoroBodyOp returned an empty ValueRange unconditionally and declare no results, and AwaitOp returned region block arguments but carries NoRegionArguments, so those ranges are always empty. Add a CIR_RegionBranchOpBase ODS class that declares the method and generates the single shared body through extraClassDefinition, mirroring the existing CIR_LoopOpBase, and retarget all ten ops onto it. The generated CIROps.h.inc is unchanged and CIROps.cpp.inc gains exactly the ten definitions removed from CIRDialect.cpp. --- clang/include/clang/CIR/Dialect/IR/CIROps.td | 48 ++++++++++------- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 57 -------------------- 2 files changed, 28 insertions(+), 77 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 5c2c948742f08..d79b17bbcd99d 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 //===----------------------------------------------------------------------===// @@ -983,8 +1001,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"; @@ -1275,8 +1292,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 ]> { @@ -1373,8 +1389,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 ]> { @@ -1515,8 +1530,7 @@ def CIR_CaseOpKind : CIR_I32EnumAttr<"CaseOpKind", "case kind", [ I32EnumAttrCase<"Range", 3, "range"> ]>; -def CIR_CaseOp : CIR_Op<"case", [ - DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>, +def CIR_CaseOp : CIR_RegionBranchOpBase<"case", [ RecursivelySpeculatable, AutomaticAllocationScope ]> { let summary = "Case operation"; @@ -1552,9 +1566,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 ]> { @@ -3220,8 +3233,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"; @@ -3330,8 +3342,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 @@ -4720,8 +4731,7 @@ def CIR_AwaitKind : CIR_I32EnumAttr<"AwaitKind", "await kind", [ I32EnumAttrCase<"Final", 3, "final"> ]>; -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"; @@ -4806,8 +4816,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 ]> { @@ -8125,8 +8134,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
