https://github.com/bcardosolopes updated https://github.com/llvm/llvm-project/pull/227127
>From a570b4355bb9901bdd7f1c6a05098be6cf996d81 Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes <[email protected]> Date: Fri, 25 Sep 2026 16:50:14 -0700 Subject: [PATCH] [CIR][mlir] Use cached symbol tables for comdat lookups Every function or global with a comdat looked its selector up by name with a linear scan of the comdat region, making CIR to LLVM lowering quadratic in the number of comdats. This happened in three places: - CIR's lowering of functions and globals: use the patterns' shared SymbolTableCollection. - The LLVM dialect comdat verifier, run on every function and global in each verification: check the comdat in verifySymbolUses, which gets a SymbolTableCollection. - ModuleTranslation: use its existing SymbolTableCollection. On the most expensive self-host TUs this cuts lowering from 30-96 s to 8-11 s and total compile time by 18-61%. Output is unchanged. --- .../CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp | 18 +++++++++++------- mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 6 ++++-- mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 19 +++++++++++-------- mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 4 ++-- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index 4bdbe38df24e8..1583b5b6f7b8e 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -3207,10 +3207,10 @@ mlir::LogicalResult CIRToLLVMGlobalOpLowering::matchAndRewrite( return mlir::success(); } -static mlir::SymbolRefAttr getComdatAttrHelper(mlir::ModuleOp modOp, - mlir::OpBuilder &builder, - StringRef symName, - mlir::LLVM::ComdatOp &comdatOp) { +static mlir::SymbolRefAttr +getComdatAttrHelper(mlir::ModuleOp modOp, mlir::OpBuilder &builder, + StringRef symName, mlir::LLVM::ComdatOp &comdatOp, + mlir::SymbolTableCollection &symbolTables) { mlir::OpBuilder::InsertionGuard guard(builder); StringRef comdatName = "__llvm_comdat"; if (!comdatOp) { @@ -3227,8 +3227,11 @@ static mlir::SymbolRefAttr getComdatAttrHelper(mlir::ModuleOp modOp, mlir::LLVM::ComdatOp::create(builder, modOp.getLoc(), comdatName); } + // Cached, and shared by both patterns: a linear scan of the comdat region + // per symbol is quadratic in the number of comdats. + mlir::SymbolTable &selectors = symbolTables.getSymbolTable(comdatOp); if (auto comdatSelector = - comdatOp.lookupSymbol<mlir::LLVM::ComdatSelectorOp>(symName)) { + selectors.lookup<mlir::LLVM::ComdatSelectorOp>(symName)) { return mlir::SymbolRefAttr::get( builder.getContext(), comdatName, mlir::FlatSymbolRefAttr::get(comdatSelector.getSymNameAttr())); @@ -3238,6 +3241,7 @@ static mlir::SymbolRefAttr getComdatAttrHelper(mlir::ModuleOp modOp, auto selectorOp = mlir::LLVM::ComdatSelectorOp::create( builder, comdatOp.getLoc(), symName, mlir::LLVM::comdat::Comdat::Any, /*sym_visibility=*/nullptr); + selectors.insert(selectorOp); return mlir::SymbolRefAttr::get( builder.getContext(), comdatName, mlir::FlatSymbolRefAttr::get(selectorOp.getSymNameAttr())); @@ -3249,7 +3253,7 @@ CIRToLLVMGlobalOpLowering::getComdatAttr(cir::GlobalOp &op, if (!op.getComdat()) return mlir::SymbolRefAttr{}; return getComdatAttrHelper(op->getParentOfType<mlir::ModuleOp>(), builder, - op.getSymName(), comdatOp); + op.getSymName(), comdatOp, symbolTables); } mlir::SymbolRefAttr @@ -3260,7 +3264,7 @@ CIRToLLVMFuncOpLowering::getComdatAttr(cir::FuncOp &op, return mlir::SymbolRefAttr{}; llvm::StringRef comdatKey = comdat->empty() ? op.getSymName() : *comdat; return getComdatAttrHelper(op->getParentOfType<mlir::ModuleOp>(), builder, - comdatKey, comdatOp); + comdatKey, comdatOp, symbolTables); } mlir::LogicalResult CIRToLLVMSwitchFlatOpLowering::matchAndRewrite( diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td index 2df02fae26d20..a50a29bfc38ba 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -1416,7 +1416,8 @@ def LLVM_AddressOfOp : LLVM_Op<"mlir.addressof", def LLVM_GlobalOp : LLVM_Op<"mlir.global", [IsolatedFromAbove, SingleBlockImplicitTerminator<"ReturnOp">, SymbolName, - SymbolVisibility, Symbol]> { + SymbolVisibility, Symbol, + DeclareOpInterfaceMethods<SymbolUserOpInterface>]> { let arguments = (ins TypeAttr:$global_type, UnitAttr:$constant, @@ -2067,7 +2068,8 @@ def LLVM_ComdatOp def LLVM_LLVMFuncOp : LLVM_Op<"func", [ AffineScope, AutomaticAllocationScope, IsolatedFromAbove, SymbolName, - SymbolVisibility, FunctionOpInterface + SymbolVisibility, FunctionOpInterface, + DeclareOpInterfaceMethods<SymbolUserOpInterface> ]> { let summary = "LLVM dialect function."; diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index ff75b5a7dfb2e..89bbb4c642112 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -2539,11 +2539,12 @@ void GlobalOp::print(OpAsmPrinter &p) { } static LogicalResult verifyComdat(Operation *op, - std::optional<SymbolRefAttr> attr) { + std::optional<SymbolRefAttr> attr, + SymbolTableCollection &symbolTable) { if (!attr) return success(); - auto *comdatSelector = SymbolTable::lookupNearestSymbolFrom(op, *attr); + auto *comdatSelector = symbolTable.lookupNearestSymbolFrom(op, *attr); if (!isa_and_nonnull<ComdatSelectorOp>(comdatSelector)) return op->emitError() << "expected comdat symbol"; @@ -2704,6 +2705,10 @@ static bool isZeroAttribute(Attribute value) { return false; } +LogicalResult GlobalOp::verifySymbolUses(SymbolTableCollection &symbolTable) { + return verifyComdat(*this, getComdat(), symbolTable); +} + LogicalResult GlobalOp::verify() { bool validType = isCompatibleOuterType(getType()) ? !llvm::isa<LLVMVoidType, TokenType, LLVMMetadataType, @@ -2754,9 +2759,6 @@ LogicalResult GlobalOp::verify() { } } - if (failed(verifyComdat(*this, getComdat()))) - return failure(); - std::optional<uint64_t> alignAttr = getAlignment(); if (alignAttr.has_value()) { uint64_t value = alignAttr.value(); @@ -3377,6 +3379,10 @@ void LLVMFuncOp::print(OpAsmPrinter &p) { } } +LogicalResult LLVMFuncOp::verifySymbolUses(SymbolTableCollection &symbolTable) { + return verifyComdat(*this, getComdat(), symbolTable); +} + // Verifies LLVM- and implementation-specific properties of the LLVM func Op: // - functions don't have 'common' linkage // - external functions have 'external' or 'extern_weak' linkage; @@ -3387,9 +3393,6 @@ LogicalResult LLVMFuncOp::verify() { << stringifyLinkage(LLVM::Linkage::Common) << "' linkage"; - if (failed(verifyComdat(*this, getComdat()))) - return failure(); - if (isExternal()) { if (getFunctionEntryCountAttr()) return emitOpError() << "external functions cannot have " diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp index bd11a493f4bd3..c07765076c5c8 100644 --- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp @@ -1249,7 +1249,7 @@ LogicalResult ModuleTranslation::convertGlobalsAndAliases() { if (std::optional<mlir::SymbolRefAttr> comdat = op.getComdat()) { auto selectorOp = cast<ComdatSelectorOp>( - SymbolTable::lookupNearestSymbolFrom(op, *comdat)); + symbolTable().lookupNearestSymbolFrom(op, *comdat)); var->setComdat(comdatMapping.lookup(selectorOp)); } @@ -2187,7 +2187,7 @@ LogicalResult ModuleTranslation::convertFunctionSignatures() { // Convert the comdat attribute. if (std::optional<mlir::SymbolRefAttr> comdat = function.getComdat()) { auto selectorOp = cast<ComdatSelectorOp>( - SymbolTable::lookupNearestSymbolFrom(function, *comdat)); + symbolTable().lookupNearestSymbolFrom(function, *comdat)); llvmFunc->setComdat(comdatMapping.lookup(selectorOp)); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
