https://github.com/OCHyams updated https://github.com/llvm/llvm-project/pull/217042
>From a9dfb36285b02a390d389ff215a49be81db80ee6 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Fri, 14 Aug 2026 13:27:34 +0100 Subject: [PATCH 01/10] Disentangle DISubprogram ODR type method uniquing from metadata uniquing Disentangle ODR type method uniquing from LLVM's general metadata uniquing. Do this by forcing the problem into debug-info creation utilities. This follows precedent of the DITypeMap model, which is a map owned by the context, enabled/disabled during IR/bitcode loading, primarily for LTO. For most metadata types, LLVM merges `uniqued` (not `distinct` or temporary) instances that are structurally identical, generally called uniquing. This happens at an LLVMContext level, rather than at a module level, meaning multiple modules can end up sharing metadata instances between them (which is fine). For DISubprogram and DIDerivedType metadata, LLVM uniques based on a subset of the fields. That means two non-identical instances may be collapsed to the same instance. This is done purposefully in order to reduce unecessary debug info duplication of ODR-type-related metadata, but it can cause issues, for example by making distinct metadata from one module reachable from another. In https://discourse.llvm.org/t//91328 this occurs during a module clone, producing invalid IR that trips the verifier. This behaviour is enabled by the map key type `MDNodeSubsetEqualImpl`, and because it's baked into the normal metata uniquing process, there's no way to "turn it off" during cloning. Ideally, LLVM would provide a mechanism to apply ODR-uniquing separetely from structural uniquing, which is what this patch aims to move towards. LLVM already partially does this. DICompositeTypes have a kind of ODR-uniquing path that is controlled via `enable/disableDebugTypeODRUniquing`, which applies similar (but annoyingly, mechanically different) ODR-uniquing at IR/bitcode load time. I've made this patch a touch gnarlier than perhaps necesssary due to combining the new DISubprogram ODR uniquing with the existing DICompositeType uniquing facility. I'm attempting to fuse these things together because they are conceptually similar and it would be simpler if we could treat them as all part of the same mechanism. Without this patch DISubprogram ODR uniquing is "always on", but it now only occurs while `isisODRUniquingDebugTypes` returns true (after `enableDebugTypeODRUniquing` is called). This is achieved with a map that's optionally used to deduplicate, while removing `MDNodeSubsetEqualImpl` to prevent any automatic ODR uniquing for DISubprograms. In order to preserve clang's debug info output this means adding DISubprogram ODR uniquing to DIBuilder, and adding a call to `enableDebugTypeODRUniquing` in CGDebugInfo for normal compilation. NOTE I have not adjusted DIBuilder to apply the DICompositeType ODR uniquing, but I think we should explore doing so in the future to continue to unify these currently separete features. ALSO NOTE I have not moved the `DIDerivedType` uniquing that is similar to the existing `DISubprogram` ODR-uniquging mechanism to the new DebugInfoODRUniquer, which I think we should also do soon in another patch. Without this patch, some tools opt-into enableDebugTypeODRUniquing: * opt, unless -disable-debug-info-type-map * clang, for thinLTO in CodeGenAction::loadModule * lto.cpp The verifier consults `isisODRUniquingDebugTypes` (seemingly as a proxy for "are we linking IR modules?"), and `UpgradeDebugInfo` checks verifier output, meaning the verifier and `UpgradeDebugInfo` act differently under those tools/settings than others. This patch adds `enableDebugTypeODRUniquing` to: * clang, for normal builds (mentioned above) * llvm-as, unless -disable-debug-info-type-map * llvm-dis, unless -disable-debug-info-type-map Meaning tests need to be wrangled to either accept the new default, typically resulting in some DICompositeTypes becoming distinct or using -disable-debug-info-type-map to preserve the old behaviour (noting this then disables the previously-always-on DISubprogram uniquing). This is a very long winded patch to prevent CloneModule from performing the ODR-type-method uniquing that currently (without the patch) occurs automatically in LLVM when uniquing takes place. Compile time tracker shows no file-size differences with this applied (nor performance regressions): https://llvm-compile-time-tracker.com/compare.php?from=d2bd0203bee02681b0a150fb8d2d6563b7e56b2e&to=52362ffb8e2d0fef740757b18c1fdfd015e3bbd1&stat=size-file --- clang/lib/CodeGen/CGDebugInfo.cpp | 2 + llvm/include/llvm/IR/DebugInfoODRUniquer.h | 110 ++++++++++++++++++ llvm/include/llvm/IR/LLVMContext.h | 2 + llvm/lib/AsmParser/LLParser.cpp | 30 +++-- llvm/lib/Bitcode/Reader/MetadataLoader.cpp | 67 ++++++----- llvm/lib/IR/CMakeLists.txt | 1 + llvm/lib/IR/DIBuilder.cpp | 43 +++++-- llvm/lib/IR/DebugInfoMetadata.cpp | 6 +- llvm/lib/IR/DebugInfoODRUniquer.cpp | 34 ++++++ llvm/lib/IR/LLVMContext.cpp | 14 ++- llvm/lib/IR/LLVMContextImpl.h | 44 +------ llvm/test/Assembler/debug-info.ll | 3 +- .../Assembler/debug-variant-discriminator.ll | 2 +- .../test/Assembler/dicompositetype-members.ll | 2 +- llvm/test/Bitcode/dityperefs-3.8.ll | 6 +- llvm/test/Linker/dicompositetype-unique.ll | 4 +- .../test/Verifier/dbg-orphaned-compileunit.ll | 2 +- .../Verifier/llvm.loop-cu-strip-followup.ll | 2 +- .../Verifier/llvm.loop-cu-strip-indirect.ll | 4 +- llvm/test/Verifier/llvm.loop-cu-strip.ll | 4 +- .../test/tools/opt/dicompositetype-members.ll | 61 ++++++++++ llvm/tools/llvm-as/llvm-as.cpp | 15 +++ llvm/tools/llvm-dis/llvm-dis.cpp | 7 ++ .../Transforms/Utils/CloningTest.cpp | 80 +++++++++++++ llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn | 1 + 25 files changed, 441 insertions(+), 105 deletions(-) create mode 100644 llvm/include/llvm/IR/DebugInfoODRUniquer.h create mode 100644 llvm/lib/IR/DebugInfoODRUniquer.cpp create mode 100644 llvm/test/tools/opt/dicompositetype-members.ll diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 27db6a3110695dc..194ee23d44584ff 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -148,6 +148,7 @@ CGDebugInfo::CGDebugInfo(CodeGenModule &CGM) : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()), DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs), DBuilder(CGM.getModule()) { + CGM.getLLVMContext().enableDebugTypeODRUniquing(); CreateCompileUnit(); } @@ -6697,6 +6698,7 @@ void CGDebugInfo::finalize() { DBuilder.retainType(cast<llvm::DIType>(MD)); DBuilder.finalize(); + CGM.getLLVMContext().disableDebugTypeODRUniquing(); } // Don't ignore in case of explicit cast where it is referenced indirectly. diff --git a/llvm/include/llvm/IR/DebugInfoODRUniquer.h b/llvm/include/llvm/IR/DebugInfoODRUniquer.h new file mode 100644 index 000000000000000..1d568c85b0c1c8e --- /dev/null +++ b/llvm/include/llvm/IR/DebugInfoODRUniquer.h @@ -0,0 +1,110 @@ +//===- llvm/IR/DebugInfoODRUniquer.h - Debug info metadata ------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Defines a class used to merge debug info for ODR types. +// +//===----------------------------------------------------------------------===// + +#include "llvm/ADT/DenseMap.h" +#include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/Metadata.h" + +namespace llvm { +class MDString; +class Metadata; +class DICompositeType; + +/// Dense set/map find_as key for use alongside DISubprogramODRInfo to +/// merge function declarations of ODR types. +struct DISubprogramODRKey { + Metadata *Scope; + StringRef LinkageName; + // TODO: Can we remove TemplateParams? + Metadata *TemplateParams; + + DISubprogramODRKey(Metadata *Scope, StringRef LinkageName, Metadata *Type, + Metadata *TemplateParams) + : Scope(Scope), LinkageName(LinkageName), TemplateParams(TemplateParams) { + } + DISubprogramODRKey(const DISubprogram *SP) + : Scope(SP->getRawScope()), LinkageName(SP->getLinkageName()), + TemplateParams(SP->getRawTemplateParams()) {} + + static bool isEqual(const DISubprogramODRKey &LHS, const DISubprogram *RHS) { + if (!LHS.Scope || LHS.LinkageName.empty()) + return false; + auto *CT = dyn_cast_or_null<DICompositeType>(LHS.Scope); + if (!CT || !CT->getRawIdentifier()) + return false; + + if (!RHS->getRawLinkageName()) + return false; + + return LHS.Scope == RHS->getRawScope() && + LHS.LinkageName == RHS->getLinkageName() && + LHS.TemplateParams == RHS->getRawTemplateParams(); + } + + static bool isEqual(const DISubprogram *LHS, const DISubprogram *RHS) { + assert(!LHS->isDefinition() && !RHS->isDefinition()); + return isEqual(DISubprogramODRKey(LHS), RHS); + } +}; + +/// Dense set/map info to merge function declarations of ODR types. +struct DISubprogramODRInfo { + static unsigned getHashValue(const DISubprogramODRKey &SP) { + // TODO: Evaluate LinkageName hash speed. + return hash_combine(SP.Scope, SP.LinkageName, SP.TemplateParams); + } + + static bool isEqual(const DISubprogramODRKey &LHS, const DISubprogram *RHS) { + if (!LHS.Scope || LHS.LinkageName.empty()) + return false; + auto *CT = dyn_cast_or_null<DICompositeType>(LHS.Scope); + if (!CT || !CT->getRawIdentifier()) + return false; + + if (!RHS->getRawLinkageName()) + return false; + + return LHS.Scope == RHS->getRawScope() && + LHS.LinkageName == RHS->getLinkageName() && + LHS.TemplateParams == RHS->getRawTemplateParams(); + } + + static bool isEqual(const DISubprogram *LHS, const DISubprogram *RHS) { + assert(!LHS->isDefinition() && !RHS->isDefinition()); + return isEqual(DISubprogramODRKey(LHS), RHS); + } +}; + +class DebugInfoODRUniquer { + /// Function declarations keyed by DISubprogramODRKey to unique on a subset + /// of fields, rather than the built-in metadata uniquing which requires + /// full structural equality. + DenseSet<DISubprogram *, DISubprogramODRInfo> FnDecls; + +public: + // FIXME: Improve the interface for types. + DenseMap<const MDString *, DICompositeType *> DITypeMap; + + /// Get an existing DISubprogram declaration with matching scope, linkage + /// name, type, and template parameters, that has been registered with + /// `addSubprogramDecl`, or return nullptr. + LLVM_ABI DISubprogram *getODRSubprogramDecl(Metadata *Scope, + StringRef LinkageName, + Metadata *Type, + Metadata *TemplateParams); + + /// Register function declaration DISubprogram, which may be reused in place + /// of other ODR-similar DISubprograms (using `getODRSubprogramDecl`). + LLVM_ABI void addSubprogramDecl(DISubprogram *SP); +}; + +} // namespace llvm diff --git a/llvm/include/llvm/IR/LLVMContext.h b/llvm/include/llvm/IR/LLVMContext.h index ced9b9227538e43..3571496e3326809 100644 --- a/llvm/include/llvm/IR/LLVMContext.h +++ b/llvm/include/llvm/IR/LLVMContext.h @@ -27,6 +27,7 @@ namespace llvm { class DiagnosticInfo; enum DiagnosticSeverity : char; +class DebugInfoODRUniquer; class Function; class Instruction; class LLVMContextImpl; @@ -164,6 +165,7 @@ class LLVMContext { LLVM_ABI bool isODRUniquingDebugTypes() const; LLVM_ABI void enableDebugTypeODRUniquing(); LLVM_ABI void disableDebugTypeODRUniquing(); + LLVM_ABI DebugInfoODRUniquer *getDebugTypeODRUniquer(); /// generateMachineFunctionNum - Get a unique number for MachineFunction /// that associated with the given Function. diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 93a79a7035e6f70..ac55c8cee9d1107 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -29,6 +29,7 @@ #include "llvm/IR/ConstantRangeList.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/DebugInfoODRUniquer.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Function.h" #include "llvm/IR/GlobalIFunc.h" @@ -6330,13 +6331,28 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) { return error( Loc, "missing 'distinct', required for !DISubprogram that is a Definition"); - Result = GET_OR_DISTINCT( - DISubprogram, - (Context, scope.Val, name.Val, linkageName.Val, file.Val, line.Val, - type.Val, scopeLine.Val, containingType.Val, virtualIndex.Val, - thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val, - declaration.Val, retainedNodes.Val, thrownTypes.Val, annotations.Val, - targetFuncName.Val, keyInstructions.Val)); + + Result = nullptr; + bool MaybeODRUnique = Context.isODRUniquingDebugTypes() && !IsDistinct && + !(SPFlags & DISubprogram::SPFlagDefinition) && + linkageName.Val; + + if (MaybeODRUnique) + Result = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( + scope.Val, linkageName.Val->getString(), type.Val, templateParams.Val); + + if (!Result) + Result = GET_OR_DISTINCT( + DISubprogram, + (Context, scope.Val, name.Val, linkageName.Val, file.Val, line.Val, + type.Val, scopeLine.Val, containingType.Val, virtualIndex.Val, + thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val, + declaration.Val, retainedNodes.Val, thrownTypes.Val, annotations.Val, + targetFuncName.Val, keyInstructions.Val)); + + if (MaybeODRUnique) + Context.getDebugTypeODRUniquer()->addSubprogramDecl( + cast<DISubprogram>(Result)); if (IsDistinct) NewDistinctSPs.push_back(cast<DISubprogram>(Result)); diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp index 8b7beb1a8ff9ece..d77ffc003818151 100644 --- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp +++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp @@ -30,6 +30,7 @@ #include "llvm/IR/BasicBlock.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/DebugInfoODRUniquer.h" #include "llvm/IR/Function.h" #include "llvm/IR/GlobalObject.h" #include "llvm/IR/GlobalVariable.h" @@ -2050,35 +2051,49 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata( } Metadata *CUorFn = getMDOrNull(Record[12 + OffsetB]); - DISubprogram *SP = GET_OR_DISTINCT( - DISubprogram, - (Context, - getDITypeRefOrNull(Record[1]), // scope - getMDString(Record[2]), // name - getMDString(Record[3]), // linkageName - getMDOrNull(Record[4]), // file - Record[5], // line - getMDOrNull(Record[6]), // type - Record[7 + OffsetA], // scopeLine - getDITypeRefOrNull(Record[8 + OffsetA]), // containingType - Record[10 + OffsetA], // virtualIndex - HasThisAdj ? Record[16 + OffsetB] : 0, // thisAdjustment - Flags, // flags - SPFlags, // SPFlags - HasUnit ? CUorFn : nullptr, // unit - getMDOrNull(Record[13 + OffsetB]), // templateParams - getMDOrNull(Record[14 + OffsetB]), // declaration - getMDOrNull(Record[15 + OffsetB]), // retainedNodes - HasThrownTypes ? getMDOrNull(Record[17 + OffsetB]) - : nullptr, // thrownTypes - HasAnnotations ? getMDOrNull(Record[18 + OffsetB]) - : nullptr, // annotations - HasTargetFuncName ? getMDString(Record[19 + OffsetB]) - : nullptr, // targetFuncName - UsesKeyInstructions)); + + DISubprogram *SP = nullptr; + bool MaybeODRUnique = Context.isODRUniquingDebugTypes() && !IsDistinct && + !(SPFlags & DISubprogram::SPFlagDefinition) && + getMDString(Record[3]); + if (MaybeODRUnique) + SP = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( + getDITypeRefOrNull(Record[1]), getMDString(Record[3])->getString(), + getMDOrNull(Record[6]), getMDOrNull(Record[13 + OffsetB])); + + if (!SP) + SP = GET_OR_DISTINCT( + DISubprogram, + (Context, + getDITypeRefOrNull(Record[1]), // scope + getMDString(Record[2]), // name + getMDString(Record[3]), // linkageName + getMDOrNull(Record[4]), // file + Record[5], // line + getMDOrNull(Record[6]), // type + Record[7 + OffsetA], // scopeLine + getDITypeRefOrNull(Record[8 + OffsetA]), // containingType + Record[10 + OffsetA], // virtualIndex + HasThisAdj ? Record[16 + OffsetB] : 0, // thisAdjustment + Flags, // flags + SPFlags, // SPFlags + HasUnit ? CUorFn : nullptr, // unit + getMDOrNull(Record[13 + OffsetB]), // templateParams + getMDOrNull(Record[14 + OffsetB]), // declaration + getMDOrNull(Record[15 + OffsetB]), // retainedNodes + HasThrownTypes ? getMDOrNull(Record[17 + OffsetB]) + : nullptr, // thrownTypes + HasAnnotations ? getMDOrNull(Record[18 + OffsetB]) + : nullptr, // annotations + HasTargetFuncName ? getMDString(Record[19 + OffsetB]) + : nullptr, // targetFuncName + UsesKeyInstructions)); MetadataList.assignValue(SP, NextMetadataNo); NextMetadataNo++; + if (MaybeODRUnique) + Context.getDebugTypeODRUniquer()->addSubprogramDecl(SP); + if (IsDistinct) NewDistinctSPs.push_back(SP); diff --git a/llvm/lib/IR/CMakeLists.txt b/llvm/lib/IR/CMakeLists.txt index 3037f01083308b5..21f21ceb229da94 100644 --- a/llvm/lib/IR/CMakeLists.txt +++ b/llvm/lib/IR/CMakeLists.txt @@ -20,6 +20,7 @@ add_llvm_component_library(LLVMCore DataLayout.cpp DebugInfo.cpp DebugInfoMetadata.cpp + DebugInfoODRUniquer.cpp DIExpressionOptimizer.cpp DebugProgramInstruction.cpp DebugLoc.cpp diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp index 4ad668866d7ef77..830681fbee65e32 100644 --- a/llvm/lib/IR/DIBuilder.cpp +++ b/llvm/lib/IR/DIBuilder.cpp @@ -1061,11 +1061,22 @@ DISubprogram *DIBuilder::createFunction( DITypeArray ThrownTypes, DINodeArray Annotations, StringRef TargetFuncName, bool UseKeyInstructions) { bool IsDefinition = SPFlags & DISubprogram::SPFlagDefinition; - auto *Node = getSubprogram( - /*IsDistinct=*/IsDefinition, VMContext, getNonCompileUnitScope(Context), - Name, LinkageName, File, LineNo, Ty, ScopeLine, nullptr, 0, 0, Flags, - SPFlags, IsDefinition ? CUNode : nullptr, TParams, Decl, nullptr, - ThrownTypes, Annotations, TargetFuncName, UseKeyInstructions); + + DISubprogram *Node = nullptr; + // Look up ODR definition if requested. + if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) + Node = VMContext.getDebugTypeODRUniquer()->getODRSubprogramDecl( + Context, LinkageName, Ty, TParams.get()); + // Otherwise or if unable, create it. + if (!Node) + Node = getSubprogram( + /*IsDistinct=*/IsDefinition, VMContext, getNonCompileUnitScope(Context), + Name, LinkageName, File, LineNo, Ty, ScopeLine, nullptr, 0, 0, Flags, + SPFlags, IsDefinition ? CUNode : nullptr, TParams, Decl, nullptr, + ThrownTypes, Annotations, TargetFuncName, UseKeyInstructions); + + if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) + VMContext.getDebugTypeODRUniquer()->addSubprogramDecl(Node); AllSubprograms.push_back(Node); trackIfUnresolved(Node); @@ -1098,11 +1109,23 @@ DISubprogram *DIBuilder::createMethod( "the compile unit."); // FIXME: Do we want to use different scope/lines? bool IsDefinition = SPFlags & DISubprogram::SPFlagDefinition; - auto *SP = getSubprogram( - /*IsDistinct=*/IsDefinition, VMContext, cast<DIScope>(Context), Name, - LinkageName, F, LineNo, Ty, LineNo, VTableHolder, VIndex, ThisAdjustment, - Flags, SPFlags, IsDefinition ? CUNode : nullptr, TParams, nullptr, - nullptr, ThrownTypes, nullptr, "", IsDefinition && UseKeyInstructions); + + DISubprogram *SP = nullptr; + // Look up ODR definition if requested. + if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) + SP = VMContext.getDebugTypeODRUniquer()->getODRSubprogramDecl( + Context, LinkageName, Ty, TParams.get()); + // Otherwise or if unable, create it. + if (!SP) + SP = getSubprogram( + /*IsDistinct=*/IsDefinition, VMContext, cast<DIScope>(Context), Name, + LinkageName, F, LineNo, Ty, LineNo, VTableHolder, VIndex, + ThisAdjustment, Flags, SPFlags, IsDefinition ? CUNode : nullptr, + TParams, nullptr, nullptr, ThrownTypes, nullptr, "", + IsDefinition && UseKeyInstructions); + + if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) + VMContext.getDebugTypeODRUniquer()->addSubprogramDecl(SP); AllSubprograms.push_back(SP); trackIfUnresolved(SP); diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp index 134f1382dfc258a..cc3ecd269d3afb7 100644 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -1079,7 +1079,7 @@ DICompositeType *DICompositeType::buildODRType( assert(!Identifier.getString().empty() && "Expected valid identifier"); if (!Context.isODRUniquingDebugTypes()) return nullptr; - auto *&CT = (*Context.pImpl->DITypeMap)[&Identifier]; + auto *&CT = Context.pImpl->ODRUniquer->DITypeMap[&Identifier]; if (!CT) return CT = DICompositeType::getDistinct( Context, Tag, Name, File, Line, Scope, BaseType, SizeInBits, @@ -1123,7 +1123,7 @@ DICompositeType *DICompositeType::getODRType( assert(!Identifier.getString().empty() && "Expected valid identifier"); if (!Context.isODRUniquingDebugTypes()) return nullptr; - auto *&CT = (*Context.pImpl->DITypeMap)[&Identifier]; + auto *&CT = Context.pImpl->ODRUniquer->DITypeMap[&Identifier]; if (!CT) { CT = DICompositeType::getDistinct( Context, Tag, Name, File, Line, Scope, BaseType, SizeInBits, @@ -1143,7 +1143,7 @@ DICompositeType *DICompositeType::getODRTypeIfExists(LLVMContext &Context, assert(!Identifier.getString().empty() && "Expected valid identifier"); if (!Context.isODRUniquingDebugTypes()) return nullptr; - return Context.pImpl->DITypeMap->lookup(&Identifier); + return Context.pImpl->ODRUniquer->DITypeMap.lookup(&Identifier); } DISubroutineType::DISubroutineType(LLVMContext &C, StorageType Storage, DIFlags Flags, uint8_t CC, diff --git a/llvm/lib/IR/DebugInfoODRUniquer.cpp b/llvm/lib/IR/DebugInfoODRUniquer.cpp new file mode 100644 index 000000000000000..a2d152c5331b4a0 --- /dev/null +++ b/llvm/lib/IR/DebugInfoODRUniquer.cpp @@ -0,0 +1,34 @@ +//===- llvm/IR/DebugInfoODRUniquer.cpp - Debug Information Builder --------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Defines a class used to merge debug info for ODR types. +// +//===----------------------------------------------------------------------===// + +#include "llvm/IR/DebugInfoODRUniquer.h" +#include "llvm/IR/DebugInfoMetadata.h" + +using namespace llvm; + +DISubprogram * +DebugInfoODRUniquer::getODRSubprogramDecl(Metadata *Scope, + StringRef LinkageName, Metadata *Type, + Metadata *TemplateParams) { + auto R = FnDecls.find_as( + DISubprogramODRKey(Scope, LinkageName, Type, TemplateParams)); + if (R == FnDecls.end()) + return nullptr; + assert(!(*R)->isDefinition() && "definition unexpectedly ODR-uniqued"); + return *R; +} + +void DebugInfoODRUniquer::addSubprogramDecl(DISubprogram *SP) { + assert(!SP->isDefinition() && + "only expect declarations DISubprogram ODR uniquing"); + FnDecls.insert(SP); +} diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp index 3e1ad638597afee..c44ca1e806fbc99 100644 --- a/llvm/lib/IR/LLVMContext.cpp +++ b/llvm/lib/IR/LLVMContext.cpp @@ -335,16 +335,22 @@ bool LLVMContext::shouldDiscardValueNames() const { return pImpl->DiscardValueNames; } -bool LLVMContext::isODRUniquingDebugTypes() const { return !!pImpl->DITypeMap; } +bool LLVMContext::isODRUniquingDebugTypes() const { + return !!pImpl->ODRUniquer; +} void LLVMContext::enableDebugTypeODRUniquing() { - if (pImpl->DITypeMap) + if (pImpl->ODRUniquer) return; - pImpl->DITypeMap.emplace(); + pImpl->ODRUniquer.emplace(); +} + +DebugInfoODRUniquer *LLVMContext::getDebugTypeODRUniquer() { + return &*pImpl->ODRUniquer; } -void LLVMContext::disableDebugTypeODRUniquing() { pImpl->DITypeMap.reset(); } +void LLVMContext::disableDebugTypeODRUniquing() { pImpl->ODRUniquer.reset(); } void LLVMContext::setDiscardValueNames(bool Discard) { pImpl->DiscardValueNames = Discard; diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h index 41c8a92c56eda06..47ccc69ce317ea9 100644 --- a/llvm/lib/IR/LLVMContextImpl.h +++ b/llvm/lib/IR/LLVMContextImpl.h @@ -29,7 +29,9 @@ #include "llvm/ADT/StringMap.h" #include "llvm/BinaryFormat/Dwarf.h" #include "llvm/IR/Constants.h" +#include "llvm/IR/DebugInfo.h" #include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/DebugInfoODRUniquer.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Metadata.h" @@ -989,46 +991,6 @@ template <> struct MDNodeKeyImpl<DISubprogram> { } }; -template <> struct MDNodeSubsetEqualImpl<DISubprogram> { - using KeyTy = MDNodeKeyImpl<DISubprogram>; - - static bool isSubsetEqual(const KeyTy &LHS, const DISubprogram *RHS) { - return isDeclarationOfODRMember(LHS.isDefinition(), LHS.Scope, - LHS.LinkageName, LHS.TemplateParams, RHS); - } - - static bool isSubsetEqual(const DISubprogram *LHS, const DISubprogram *RHS) { - return isDeclarationOfODRMember(LHS->isDefinition(), LHS->getRawScope(), - LHS->getRawLinkageName(), - LHS->getRawTemplateParams(), RHS); - } - - /// Subprograms compare equal if they declare the same function in an ODR - /// type. - static bool isDeclarationOfODRMember(bool IsDefinition, const Metadata *Scope, - const MDString *LinkageName, - const Metadata *TemplateParams, - const DISubprogram *RHS) { - // Check whether the LHS is eligible. - if (IsDefinition || !Scope || !LinkageName) - return false; - - auto *CT = dyn_cast_or_null<DICompositeType>(Scope); - if (!CT || !CT->getRawIdentifier()) - return false; - - // Compare to the RHS. - // FIXME: We need to compare template parameters here to avoid incorrect - // collisions in mapMetadata when RF_ReuseAndMutateDistinctMDs and a - // ODR-DISubprogram has a non-ODR template parameter (i.e., a - // DICompositeType that does not have an identifier). Eventually we should - // decouple ODR logic from uniquing logic. - return IsDefinition == RHS->isDefinition() && Scope == RHS->getRawScope() && - LinkageName == RHS->getRawLinkageName() && - TemplateParams == RHS->getRawTemplateParams(); - } -}; - template <> struct MDNodeKeyImpl<DILexicalBlock> { Metadata *Scope; Metadata *File; @@ -1616,7 +1578,7 @@ class LLVMContextImpl { #include "llvm/IR/Metadata.def" // Optional map for looking up composite types by identifier. - std::optional<DenseMap<const MDString *, DICompositeType *>> DITypeMap; + std::optional<DebugInfoODRUniquer> ODRUniquer; // MDNodes may be uniqued or not uniqued. When they're not uniqued, they // aren't in the MDNodeSet, but they're still shared between objects, so no diff --git a/llvm/test/Assembler/debug-info.ll b/llvm/test/Assembler/debug-info.ll index a1978ef375a9ebe..c959ef1c7f80305 100644 --- a/llvm/test/Assembler/debug-info.ll +++ b/llvm/test/Assembler/debug-info.ll @@ -1,4 +1,5 @@ -; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s +; RUN: llvm-as --disable-debug-info-type-map=true < %s | llvm-dis --disable-debug-info-type-map=true \ +; RUN: | llvm-as --disable-debug-info-type-map=true | llvm-dis --disable-debug-info-type-map=true | FileCheck %s ; RUN: verify-uselistorder %s ; CHECK: !named = !{!0, !0, !1, !2, !3, !4, !5, !6, !7, !8, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23, !24, !25, !26, !27, !27, !28, !29, !30, !31, !32, !33, !34, !35, !36, !37, !38, !39, !40, !41, !42, !43, !44, !45, !46, !47, !48} diff --git a/llvm/test/Assembler/debug-variant-discriminator.ll b/llvm/test/Assembler/debug-variant-discriminator.ll index 5be001cad6beefc..9d1291017edd842 100644 --- a/llvm/test/Assembler/debug-variant-discriminator.ll +++ b/llvm/test/Assembler/debug-variant-discriminator.ll @@ -4,7 +4,7 @@ ; CHECK: !named = !{!0, !1, !2} !named = !{!0, !1, !2} -; CHECK: !0 = !DICompositeType(tag: DW_TAG_structure_type, name: "Outer", size: 64, align: 64, identifier: "Outer") +; CHECK: !0 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Outer", size: 64, align: 64, identifier: "Outer") ; CHECK-NEXT: !1 = !DICompositeType(tag: DW_TAG_variant_part, scope: !0, size: 64, discriminator: !2) ; CHECK-NEXT: !2 = !DIDerivedType(tag: DW_TAG_member, scope: !1, baseType: !3, size: 64, align: 64, flags: DIFlagArtificial) ; CHECK-NEXT: !3 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned) diff --git a/llvm/test/Assembler/dicompositetype-members.ll b/llvm/test/Assembler/dicompositetype-members.ll index c69bc1413ae9b99..517bfbe719e2775 100644 --- a/llvm/test/Assembler/dicompositetype-members.ll +++ b/llvm/test/Assembler/dicompositetype-members.ll @@ -13,7 +13,7 @@ !2 = !DIFile(filename: "path/to/other", directory: "/path/to/dir") ; Define an identified type with fields and functions. -; CHECK-NEXT: !3 = !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid",{{.*}}, identifier: "uuid") +; CHECK-NEXT: !3 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid",{{.*}}, identifier: "uuid") ; CHECK-NEXT: !4 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !1 ; CHECK-NEXT: !5 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !3, file: !1 ; CHECK-NEXT: !6 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !1, type: !7, spFlags: 0) diff --git a/llvm/test/Bitcode/dityperefs-3.8.ll b/llvm/test/Bitcode/dityperefs-3.8.ll index 7f413a06ddb3ace..21ad9c9a0fb67f0 100644 --- a/llvm/test/Bitcode/dityperefs-3.8.ll +++ b/llvm/test/Bitcode/dityperefs-3.8.ll @@ -8,8 +8,8 @@ ; CHECK: @G1 = global i32 0 ; CHECK: !0 = !DIFile(filename: "path/to/file", directory: "/path/to/dir") -; CHECK-NEXT: !1 = !DICompositeType(tag: DW_TAG_structure_type, name: "T1"{{.*}}, identifier: "T1") -; CHECK-NEXT: !2 = !DICompositeType(tag: DW_TAG_structure_type, name: "T2", scope: !1{{.*}}, baseType: !1, vtableHolder: !1, identifier: "T2") +; CHECK-NEXT: !1 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "T1"{{.*}}, identifier: "T1") +; CHECK-NEXT: !2 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "T2", scope: !1{{.*}}, baseType: !1, vtableHolder: !1, identifier: "T2") ; CHECK-NEXT: !3 = !DIDerivedType(tag: DW_TAG_member, name: "M1", scope: !1{{.*}}, baseType: !2) ; CHECK-NEXT: !4 = !DISubroutineType(types: !5) ; CHECK-NEXT: !5 = !{!1, !2} @@ -20,7 +20,7 @@ ; CHECK-NEXT: !10 = !DIGlobalVariable(name: "G",{{.*}} type: !1, ; CHECK-NEXT: !11 = !DITemplateValueParameter(type: !1, value: ptr @G1) ; CHECK-NEXT: !12 = !DIImportedEntity(tag: DW_TAG_imported_module, name: "T2", scope: !0, entity: !1) -; CHECK-NEXT: !13 = !DICompositeType(tag: DW_TAG_structure_type, name: "T3", file: !0, elements: !14, identifier: "T3") +; CHECK-NEXT: !13 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "T3", file: !0, elements: !14, identifier: "T3") ; CHECK-NEXT: !14 = !{!15} ; CHECK-NEXT: !15 = !DISubprogram(scope: !13, ; CHECK-NEXT: !16 = !DIDerivedType(tag: DW_TAG_ptr_to_member_type,{{.*}} extraData: !13) diff --git a/llvm/test/Linker/dicompositetype-unique.ll b/llvm/test/Linker/dicompositetype-unique.ll index 28b2f33001b9e14..abc2c5613b52ee3 100644 --- a/llvm/test/Linker/dicompositetype-unique.ll +++ b/llvm/test/Linker/dicompositetype-unique.ll @@ -6,8 +6,8 @@ ; RUN: | FileCheck %s -check-prefix NOMAP ; Check that the bitcode reader handles this too. -; RUN: llvm-as -o %t1.bc <%s -; RUN: llvm-as -o %t2.bc <%S/Inputs/dicompositetype-unique.ll +; RUN: llvm-as -disable-debug-info-type-map -o %t1.bc <%s +; RUN: llvm-as -disable-debug-info-type-map -o %t2.bc <%S/Inputs/dicompositetype-unique.ll ; RUN: llvm-link -S -o - %t1.bc %t2.bc \ ; RUN: | FileCheck %s -check-prefix CHECK -check-prefix FORWARD ; RUN: llvm-link -S -o - %t2.bc %t1.bc \ diff --git a/llvm/test/Verifier/dbg-orphaned-compileunit.ll b/llvm/test/Verifier/dbg-orphaned-compileunit.ll index 9ab72824624df59..6a6a663b40596e7 100644 --- a/llvm/test/Verifier/dbg-orphaned-compileunit.ll +++ b/llvm/test/Verifier/dbg-orphaned-compileunit.ll @@ -1,4 +1,4 @@ -; RUN: not llvm-as -disable-output <%s 2>&1 | FileCheck %s +; RUN: not llvm-as --disable-debug-info-type-map=true -disable-output <%s 2>&1 | FileCheck %s ; CHECK: assembly parsed, but does not verify ; CHECK-NEXT: DICompileUnit not listed in llvm.dbg.cu ; CHECK-NEXT: !0 = distinct !DICompileUnit(language: DW_LANG_Fortran77, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug) diff --git a/llvm/test/Verifier/llvm.loop-cu-strip-followup.ll b/llvm/test/Verifier/llvm.loop-cu-strip-followup.ll index 833c76276e458ae..13ba62f47a3b430 100644 --- a/llvm/test/Verifier/llvm.loop-cu-strip-followup.ll +++ b/llvm/test/Verifier/llvm.loop-cu-strip-followup.ll @@ -1,4 +1,4 @@ -; RUN: llvm-as < %s -o - | llvm-dis - | FileCheck %s +; RUN: llvm-as < %s -o - -disable-debug-info-type-map | llvm-dis - -disable-debug-info-type-map | FileCheck %s ; The loop metadata in this test is similar to the output from clang for ; code like the this: diff --git a/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll b/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll index d724cf7de97e008..d4d9176b64c897a 100644 --- a/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll +++ b/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll @@ -1,5 +1,5 @@ -; RUN: llvm-as -disable-output < %s -o /dev/null 2>&1 | FileCheck %s -; RUN: llvm-as < %s -o - | llvm-dis - | FileCheck %s --check-prefix=CHECK-STRIP +; RUN: llvm-as -disable-output -disable-debug-info-type-map < %s -o /dev/null 2>&1 | FileCheck %s +; RUN: llvm-as < %s -o - -disable-debug-info-type-map | llvm-dis - | FileCheck %s --check-prefix=CHECK-STRIP ; CHECK: DICompileUnit not listed in llvm.dbg.cu ; CHECK: ignoring invalid debug info in ; CHECK-NOT: DICompileUnit not listed in llvm.dbg.cu diff --git a/llvm/test/Verifier/llvm.loop-cu-strip.ll b/llvm/test/Verifier/llvm.loop-cu-strip.ll index 5f7923e067fe996..426f8048fdd9dbf 100644 --- a/llvm/test/Verifier/llvm.loop-cu-strip.ll +++ b/llvm/test/Verifier/llvm.loop-cu-strip.ll @@ -1,5 +1,5 @@ -; RUN: llvm-as -disable-output < %s -o /dev/null 2>&1 | FileCheck %s -; RUN: llvm-as < %s -o - | llvm-dis - | FileCheck %s --check-prefix=CHECK-STRIP +; RUN: llvm-as -disable-output -disable-debug-info-type-map < %s -o /dev/null 2>&1 | FileCheck %s +; RUN: llvm-as -disable-debug-info-type-map < %s -o - | llvm-dis -disable-debug-info-type-map - | FileCheck %s --check-prefix=CHECK-STRIP ; CHECK: DICompileUnit not listed in llvm.dbg.cu ; CHECK: ignoring invalid debug info in ; CHECK-NOT: DICompileUnit not listed in llvm.dbg.cu diff --git a/llvm/test/tools/opt/dicompositetype-members.ll b/llvm/test/tools/opt/dicompositetype-members.ll new file mode 100644 index 000000000000000..a7fc3bd7fc68201 --- /dev/null +++ b/llvm/test/tools/opt/dicompositetype-members.ll @@ -0,0 +1,61 @@ +; RUN: opt < %s -S | FileCheck %s +; RUN: verify-uselistorder %s + +;; Copied from Assembler/dicompositetype-memebers.ll +;; Check opt applies the same odr type debug unquing. + +; Anchor the order of the nodes. +!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17} + +; Some basic building blocks. +; CHECK: !0 = !DIBasicType +; CHECK-NEXT: !1 = !DIFile +; CHECK-NEXT: !2 = !DIFile +!0 = !DIBasicType(tag: DW_TAG_base_type, name: "name", size: 1, align: 2, encoding: DW_ATE_unsigned_char) +!1 = !DIFile(filename: "path/to/file", directory: "/path/to/dir") +!2 = !DIFile(filename: "path/to/other", directory: "/path/to/dir") + +; Define an identified type with fields and functions. +; CHECK-NEXT: !3 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid",{{.*}}, identifier: "uuid") +; CHECK-NEXT: !4 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !1 +; CHECK-NEXT: !5 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !3, file: !1 +; CHECK-NEXT: !6 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !1, type: !7, spFlags: 0) +; CHECK-NEXT: !7 = !DISubroutineType(types: !8) +; CHECK-NEXT: !8 = !{null} +; CHECK-NEXT: !9 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !3, file: !1, type: !7, spFlags: 0) +!3 = !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid", file: !1, line: 2, size: 64, align: 32, identifier: "uuid") +!4 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!5 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !3, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!6 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !1, isDefinition: false, type: !18) +!7 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !3, file: !1, isDefinition: false, type: !18) +!18 = !DISubroutineType(types: !19) +!19 = !{null} + +; Define an un-identified type with fields and functions. +; CHECK-NEXT: !10 = !DICompositeType(tag: DW_TAG_structure_type, name: "no-uuid", file: !1 +; CHECK-NEXT: !11 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !10, file: !1 +; CHECK-NEXT: !12 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !10, file: !1 +; CHECK-NEXT: !13 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !1, type: !7, spFlags: 0) +; CHECK-NEXT: !14 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !10, file: !1, type: !7, spFlags: 0) +!8 = !DICompositeType(tag: DW_TAG_structure_type, name: "no-uuid", file: !1, line: 2, size: 64, align: 32) +!9 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !8, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!10 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !8, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!11 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !8, file: !1, isDefinition: false, type: !18) +!12 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !8, file: !1, isDefinition: false, type: !18) + +; Add duplicate fields and members of "no-uuid" in a different file. These +; should stick around, since "no-uuid" does not have an "identifier:" field. +; CHECK-NEXT: !15 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !10, file: !2, +; CHECK-NEXT: !16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !2, type: !7, spFlags: 0) +!13 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !8, file: !2, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!14 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !8, file: !2, isDefinition: false, type: !18) + +; Add duplicate fields and members of "has-uuid" in a different file. These +; should be merged. +!15 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !2, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !2, isDefinition: false, type: !18) + +; CHECK-NEXT: !17 = !{!4, !6} +; CHECK-NOT: !DIDerivedType +; CHECK-NOT: !DISubprogram +!17 = !{!15, !16} diff --git a/llvm/tools/llvm-as/llvm-as.cpp b/llvm/tools/llvm-as/llvm-as.cpp index 5713f2839783420..9315744b693553d 100644 --- a/llvm/tools/llvm-as/llvm-as.cpp +++ b/llvm/tools/llvm-as/llvm-as.cpp @@ -62,6 +62,10 @@ static cl::opt<std::string> ClDataLayout("data-layout", cl::value_desc("layout-string"), cl::init(""), cl::cat(AsCat)); +static cl::opt<bool> + DisableDITypeMap("disable-debug-info-type-map", + cl::desc("Don't use a uniquing type map for debug info")); + static void WriteOutputFile(const Module *M, const ModuleSummaryIndex *Index) { // Infer the output filename if needed. if (OutputFilename.empty()) { @@ -113,6 +117,9 @@ int main(int argc, char **argv) { cl::ParseCommandLineOptions(argc, argv, "llvm .ll -> .bc assembler\n"); LLVMContext Context; + if (!DisableDITypeMap) + Context.enableDebugTypeODRUniquing(); + // Parse the file now... SMDiagnostic Err; auto SetDataLayout = [](StringRef, StringRef) -> std::optional<std::string> { @@ -136,6 +143,14 @@ int main(int argc, char **argv) { std::unique_ptr<ModuleSummaryIndex> Index = std::move(ModuleAndIndex.Index); + // The verifier seems to use DebugTypeODRUniquing as an (inconsistent) proxy + // for whether module linking is taking place. To maintain previous llvm-as + // behaviour, don't leave DebugTypeODRUniquing "on" for the verifier. + // FIXME: Improve this situation, because it means opt and llvm-as have + // different verifier paths. + if (!DisableDITypeMap) + Context.disableDebugTypeODRUniquing(); + if (!DisableVerify) { std::string ErrorStr; raw_string_ostream OS(ErrorStr); diff --git a/llvm/tools/llvm-dis/llvm-dis.cpp b/llvm/tools/llvm-dis/llvm-dis.cpp index a961f9cb0f7dd9a..8b37df6ccda94d0 100644 --- a/llvm/tools/llvm-dis/llvm-dis.cpp +++ b/llvm/tools/llvm-dis/llvm-dis.cpp @@ -86,6 +86,10 @@ static cl::opt<bool> "then materialize only the metadata"), cl::cat(DisCategory)); +static cl::opt<bool> + DisableDITypeMap("disable-debug-info-type-map", + cl::desc("Don't use a uniquing type map for debug info")); + static cl::opt<bool> PrintThinLTOIndexOnly( "print-thinlto-index-only", cl::desc("Only read thinlto index and print the index as LLVM assembly."), @@ -193,6 +197,9 @@ int main(int argc, char **argv) { Context.setDiagnosticHandler( std::make_unique<LLVMDisDiagnosticHandler>(argv[0])); + if (!DisableDITypeMap) + Context.enableDebugTypeODRUniquing(); + ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr = MemoryBuffer::getFileOrSTDIN(InputFilename); if (std::error_code EC = BufferOrErr.getError()) { diff --git a/llvm/unittests/Transforms/Utils/CloningTest.cpp b/llvm/unittests/Transforms/Utils/CloningTest.cpp index 30318b02007abd3..1824a79ee655297 100644 --- a/llvm/unittests/Transforms/Utils/CloningTest.cpp +++ b/llvm/unittests/Transforms/Utils/CloningTest.cpp @@ -1440,4 +1440,84 @@ TEST_F(CloneModule, GlobalWithBlockAddressesInitializer) { ASSERT_NE(OriginalBa->getBasicBlock(), ClonedBa->getBasicBlock()); } +TEST_F(CloneModule, ODRUniqueTypeMethodVerifier) { + // Test that the following IR, reduced from a self-host clang-22 compile, + // doesn't trip the verifier after cloning. + // + // The focus of the test is "operator!=" which is a DISubprogram that is: + // * a declaration (therefore not distinct), + // * has uniqued metadata fields used for ODR-type-method uniquing + // (linkage name, scope, template params), + // * and has other distinct metadata fields (type). + // + // The issue can be reproduced from C++ from this less-reduced C++: + // ``` + // template <typename e> class BaseIt { + // public: + // [[gnu::nodebug]] BaseIt(); + // bool operator!=(e); + // }; + // + // template <typename TLambda> class It : public BaseIt<It<TLambda>> { + // public: + // It() {} + // [[gnu::nodebug]] It end(); + // [[gnu::nodebug]] It begin(); + // [[gnu::nodebug]] int *operator++(); + // [[gnu::nodebug]] int operator*(); + // }; + // + // template <typename TLambda > [[gnu::nodebug]] It<TLambda> i(TLambda) { + // return {}; + // } + // + // class ar { + // public: + // static auto lol() { + // return i([] {}); + // } + // }; + // + // [[gnu::nodebug]] ar au; + // + // bool func() { + // for ([[gnu::nodebug]] auto av : au.lol()) + // ; + // } + // ``` + + StringRef IR = R"( + declare !dbg !4 i1 @f(ptr) + + !llvm.dbg.cu = !{!0} + !llvm.module.flags = !{!3} + + !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "reduced", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !2, splitDebugInlining: false, nameTableKind: None) + !1 = !DIFile(filename: "reduced.cpp", directory: "/") + !2 = !{} + !3 = !{i32 2, !"Debug Info Version", i32 3} + !4 = !DISubprogram(name: "operator!=", linkageName: "_ZN6BaseItI2ItIZN2ar3lolEvEUlvE_EEneES3_", scope: !5, file: !1, line: 4, type: !6, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized) + !5 = !DICompositeType(tag: DW_TAG_class_type, name: "BaseIt<It<(lambda)> >", file: !1, line: 1, size: 8, flags: DIFlagFwdDecl | DIFlagNonTrivial, identifier: "_ZTS6BaseItI2ItIZN2ar3lolEvEUlvE_EE") + !6 = distinct !DISubroutineType(types: !7) + !7 = !{!8} + !8 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "It<(lambda)>", file: !1, line: 7, size: 8, flags: DIFlagTypePassByValue | DIFlagNonTrivial, elements: !2, templateParams: !9, identifier: "_ZTS2ItIZN2ar3lolEvEUlvE_E") + !9 = !{!10} + !10 = !DITemplateTypeParameter(name: "TLambda", type: !11) + !11 = distinct !DICompositeType(tag: DW_TAG_class_type, scope: !12, file: !1, line: 23, size: 8, flags: DIFlagTypePassByValue | DIFlagNonTrivial, elements: !2, identifier: "_ZTSZN2ar3lolEvEUlvE_") + !12 = distinct !DISubprogram(name: "lol", linkageName: "_ZN2ar3lolEv", scope: !13, file: !1, line: 22, type: !14, scopeLine: 22, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, declaration: !15, retainedNodes: !2, keyInstructions: true) + !13 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "ar", file: !1, line: 20, size: 8, flags: DIFlagTypePassByValue, elements: !2, identifier: "_ZTS2ar") + !14 = !DISubroutineType(types: !7) + !15 = !DISubprogram(name: "lol", linkageName: "_ZN2ar3lolEv", scope: !13, file: !1, line: 22, type: !14, scopeLine: 22, flags: DIFlagPublic | DIFlagPrototyped | DIFlagStaticMember, spFlags: DISPFlagOptimized) + )"; + + LLVMContext Context; + SMDiagnostic Error; + + std::unique_ptr<Module> M = parseAssemblyString(IR, Error, Context); + ASSERT_FALSE(verifyModule(*M, &errs())); + + std::unique_ptr<Module> Clone = llvm::CloneModule(*M); + EXPECT_FALSE(verifyModule(*Clone)); +} + } // namespace diff --git a/llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn index 1135884bdfd68e3..aadaaf77ef0bc39 100644 --- a/llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn +++ b/llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn @@ -35,6 +35,7 @@ static_library("IR") { "DataLayout.cpp", "DebugInfo.cpp", "DebugInfoMetadata.cpp", + "DebugInfoODRUniquer.cpp" "DebugLoc.cpp", "DebugProgramInstruction.cpp", "DiagnosticHandler.cpp", >From 0418c011fb79363365579da566ab3b2b6664338d Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Tue, 18 Aug 2026 16:00:25 +0100 Subject: [PATCH 02/10] avoid confusing use of the word 'definition' in comment --- llvm/lib/IR/DIBuilder.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp index 830681fbee65e32..ff273b6b3232c32 100644 --- a/llvm/lib/IR/DIBuilder.cpp +++ b/llvm/lib/IR/DIBuilder.cpp @@ -1063,7 +1063,7 @@ DISubprogram *DIBuilder::createFunction( bool IsDefinition = SPFlags & DISubprogram::SPFlagDefinition; DISubprogram *Node = nullptr; - // Look up ODR definition if requested. + // Look up ODR declaration if requested. if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) Node = VMContext.getDebugTypeODRUniquer()->getODRSubprogramDecl( Context, LinkageName, Ty, TParams.get()); @@ -1111,7 +1111,7 @@ DISubprogram *DIBuilder::createMethod( bool IsDefinition = SPFlags & DISubprogram::SPFlagDefinition; DISubprogram *SP = nullptr; - // Look up ODR definition if requested. + // Look up ODR declaration if requested. if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) SP = VMContext.getDebugTypeODRUniquer()->getODRSubprogramDecl( Context, LinkageName, Ty, TParams.get()); >From e8acb5dc65a1d9cfe44ac5fc0bae103c5cff448e Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Thu, 10 Sep 2026 11:27:12 +0100 Subject: [PATCH 03/10] Use scope identifier string in DISubprogramODRKey to avoid hashing temporary f MD node addresses --- llvm/include/llvm/IR/DebugInfoODRUniquer.h | 45 ++++++++-------------- llvm/lib/IR/DebugInfoODRUniquer.cpp | 10 ++++- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/llvm/include/llvm/IR/DebugInfoODRUniquer.h b/llvm/include/llvm/IR/DebugInfoODRUniquer.h index 1d568c85b0c1c8e..6533f36cbe6cf09 100644 --- a/llvm/include/llvm/IR/DebugInfoODRUniquer.h +++ b/llvm/include/llvm/IR/DebugInfoODRUniquer.h @@ -22,32 +22,31 @@ class DICompositeType; /// Dense set/map find_as key for use alongside DISubprogramODRInfo to /// merge function declarations of ODR types. struct DISubprogramODRKey { - Metadata *Scope; + StringRef ScopeIdentifier; StringRef LinkageName; - // TODO: Can we remove TemplateParams? - Metadata *TemplateParams; - DISubprogramODRKey(Metadata *Scope, StringRef LinkageName, Metadata *Type, - Metadata *TemplateParams) - : Scope(Scope), LinkageName(LinkageName), TemplateParams(TemplateParams) { + DISubprogramODRKey(StringRef ScopeIdentifier, StringRef LinkageName) + : ScopeIdentifier(ScopeIdentifier), LinkageName(LinkageName) {} + + DISubprogramODRKey(const DISubprogram *SP) { + auto *CT = dyn_cast_or_null<DICompositeType>(SP->getRawScope()); + ScopeIdentifier = CT ? CT->getIdentifier() : StringRef(); + LinkageName = SP->getLinkageName(); } - DISubprogramODRKey(const DISubprogram *SP) - : Scope(SP->getRawScope()), LinkageName(SP->getLinkageName()), - TemplateParams(SP->getRawTemplateParams()) {} static bool isEqual(const DISubprogramODRKey &LHS, const DISubprogram *RHS) { - if (!LHS.Scope || LHS.LinkageName.empty()) + if (LHS.LinkageName.empty() || LHS.ScopeIdentifier.empty()) return false; - auto *CT = dyn_cast_or_null<DICompositeType>(LHS.Scope); + + auto *CT = dyn_cast_or_null<DICompositeType>(RHS->getRawScope()); if (!CT || !CT->getRawIdentifier()) return false; if (!RHS->getRawLinkageName()) return false; - return LHS.Scope == RHS->getRawScope() && - LHS.LinkageName == RHS->getLinkageName() && - LHS.TemplateParams == RHS->getRawTemplateParams(); + return LHS.LinkageName == RHS->getLinkageName() && + LHS.ScopeIdentifier == CT->getIdentifier(); } static bool isEqual(const DISubprogram *LHS, const DISubprogram *RHS) { @@ -59,28 +58,16 @@ struct DISubprogramODRKey { /// Dense set/map info to merge function declarations of ODR types. struct DISubprogramODRInfo { static unsigned getHashValue(const DISubprogramODRKey &SP) { - // TODO: Evaluate LinkageName hash speed. - return hash_combine(SP.Scope, SP.LinkageName, SP.TemplateParams); + return hash_combine(SP.ScopeIdentifier, SP.LinkageName); } static bool isEqual(const DISubprogramODRKey &LHS, const DISubprogram *RHS) { - if (!LHS.Scope || LHS.LinkageName.empty()) - return false; - auto *CT = dyn_cast_or_null<DICompositeType>(LHS.Scope); - if (!CT || !CT->getRawIdentifier()) - return false; - - if (!RHS->getRawLinkageName()) - return false; - - return LHS.Scope == RHS->getRawScope() && - LHS.LinkageName == RHS->getLinkageName() && - LHS.TemplateParams == RHS->getRawTemplateParams(); + return DISubprogramODRKey::isEqual(LHS, RHS); } static bool isEqual(const DISubprogram *LHS, const DISubprogram *RHS) { assert(!LHS->isDefinition() && !RHS->isDefinition()); - return isEqual(DISubprogramODRKey(LHS), RHS); + return DISubprogramODRKey::isEqual(DISubprogramODRKey(LHS), RHS); } }; diff --git a/llvm/lib/IR/DebugInfoODRUniquer.cpp b/llvm/lib/IR/DebugInfoODRUniquer.cpp index a2d152c5331b4a0..b8ff59c8ef50eae 100644 --- a/llvm/lib/IR/DebugInfoODRUniquer.cpp +++ b/llvm/lib/IR/DebugInfoODRUniquer.cpp @@ -19,10 +19,16 @@ DISubprogram * DebugInfoODRUniquer::getODRSubprogramDecl(Metadata *Scope, StringRef LinkageName, Metadata *Type, Metadata *TemplateParams) { - auto R = FnDecls.find_as( - DISubprogramODRKey(Scope, LinkageName, Type, TemplateParams)); + // Only methods, which have a type scope, are eligable for ODR uniquing. + auto *CT = dyn_cast_or_null<DICompositeType>(Scope); + if (!CT || !CT->getRawIdentifier()) + return nullptr; + + auto R = + FnDecls.find_as(DISubprogramODRKey(CT->getIdentifier(), LinkageName)); if (R == FnDecls.end()) return nullptr; + assert(!(*R)->isDefinition() && "definition unexpectedly ODR-uniqued"); return *R; } >From fb76c5bd9f56665fc10338389cd060ff76c312b5 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Wed, 30 Sep 2026 11:02:41 +0100 Subject: [PATCH 04/10] test update, don't add every func to the map --- llvm/lib/IR/DebugInfoODRUniquer.cpp | 7 +++ .../test/Assembler/dicompositetype-members.ll | 3 + .../test/tools/opt/dicompositetype-members.ll | 61 ------------------- 3 files changed, 10 insertions(+), 61 deletions(-) delete mode 100644 llvm/test/tools/opt/dicompositetype-members.ll diff --git a/llvm/lib/IR/DebugInfoODRUniquer.cpp b/llvm/lib/IR/DebugInfoODRUniquer.cpp index b8ff59c8ef50eae..be5163cf997165f 100644 --- a/llvm/lib/IR/DebugInfoODRUniquer.cpp +++ b/llvm/lib/IR/DebugInfoODRUniquer.cpp @@ -36,5 +36,12 @@ DebugInfoODRUniquer::getODRSubprogramDecl(Metadata *Scope, void DebugInfoODRUniquer::addSubprogramDecl(DISubprogram *SP) { assert(!SP->isDefinition() && "only expect declarations DISubprogram ODR uniquing"); + if (SP->getLinkageName().empty()) + return; + // Only methods, which have a type scope, are eligable for ODR uniquing. + auto *CT = dyn_cast_or_null<DICompositeType>(SP->getScope()); + if (!CT || !CT->getRawIdentifier()) + return; + FnDecls.insert(SP); } diff --git a/llvm/test/Assembler/dicompositetype-members.ll b/llvm/test/Assembler/dicompositetype-members.ll index 517bfbe719e2775..288358abe8dded8 100644 --- a/llvm/test/Assembler/dicompositetype-members.ll +++ b/llvm/test/Assembler/dicompositetype-members.ll @@ -1,6 +1,9 @@ ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s +; RUN: opt < %s -S | FileCheck %s ; RUN: verify-uselistorder %s +;; This test also runs opt to check opt applies the same odr type debug uniquing. + ; Anchor the order of the nodes. !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17} diff --git a/llvm/test/tools/opt/dicompositetype-members.ll b/llvm/test/tools/opt/dicompositetype-members.ll deleted file mode 100644 index a7fc3bd7fc68201..000000000000000 --- a/llvm/test/tools/opt/dicompositetype-members.ll +++ /dev/null @@ -1,61 +0,0 @@ -; RUN: opt < %s -S | FileCheck %s -; RUN: verify-uselistorder %s - -;; Copied from Assembler/dicompositetype-memebers.ll -;; Check opt applies the same odr type debug unquing. - -; Anchor the order of the nodes. -!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17} - -; Some basic building blocks. -; CHECK: !0 = !DIBasicType -; CHECK-NEXT: !1 = !DIFile -; CHECK-NEXT: !2 = !DIFile -!0 = !DIBasicType(tag: DW_TAG_base_type, name: "name", size: 1, align: 2, encoding: DW_ATE_unsigned_char) -!1 = !DIFile(filename: "path/to/file", directory: "/path/to/dir") -!2 = !DIFile(filename: "path/to/other", directory: "/path/to/dir") - -; Define an identified type with fields and functions. -; CHECK-NEXT: !3 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid",{{.*}}, identifier: "uuid") -; CHECK-NEXT: !4 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !1 -; CHECK-NEXT: !5 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !3, file: !1 -; CHECK-NEXT: !6 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !1, type: !7, spFlags: 0) -; CHECK-NEXT: !7 = !DISubroutineType(types: !8) -; CHECK-NEXT: !8 = !{null} -; CHECK-NEXT: !9 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !3, file: !1, type: !7, spFlags: 0) -!3 = !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid", file: !1, line: 2, size: 64, align: 32, identifier: "uuid") -!4 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!5 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !3, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!6 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !1, isDefinition: false, type: !18) -!7 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !3, file: !1, isDefinition: false, type: !18) -!18 = !DISubroutineType(types: !19) -!19 = !{null} - -; Define an un-identified type with fields and functions. -; CHECK-NEXT: !10 = !DICompositeType(tag: DW_TAG_structure_type, name: "no-uuid", file: !1 -; CHECK-NEXT: !11 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !10, file: !1 -; CHECK-NEXT: !12 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !10, file: !1 -; CHECK-NEXT: !13 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !1, type: !7, spFlags: 0) -; CHECK-NEXT: !14 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !10, file: !1, type: !7, spFlags: 0) -!8 = !DICompositeType(tag: DW_TAG_structure_type, name: "no-uuid", file: !1, line: 2, size: 64, align: 32) -!9 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !8, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!10 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !8, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!11 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !8, file: !1, isDefinition: false, type: !18) -!12 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !8, file: !1, isDefinition: false, type: !18) - -; Add duplicate fields and members of "no-uuid" in a different file. These -; should stick around, since "no-uuid" does not have an "identifier:" field. -; CHECK-NEXT: !15 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !10, file: !2, -; CHECK-NEXT: !16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !2, type: !7, spFlags: 0) -!13 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !8, file: !2, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!14 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !8, file: !2, isDefinition: false, type: !18) - -; Add duplicate fields and members of "has-uuid" in a different file. These -; should be merged. -!15 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !2, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !2, isDefinition: false, type: !18) - -; CHECK-NEXT: !17 = !{!4, !6} -; CHECK-NOT: !DIDerivedType -; CHECK-NOT: !DISubprogram -!17 = !{!15, !16} >From 74cd0dd795c4fe100436db1185365565880a7e63 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Wed, 30 Sep 2026 14:40:01 +0100 Subject: [PATCH 05/10] rm unused params --- llvm/include/llvm/IR/DebugInfoODRUniquer.h | 4 +--- llvm/lib/AsmParser/LLParser.cpp | 2 +- llvm/lib/Bitcode/Reader/MetadataLoader.cpp | 3 +-- llvm/lib/IR/DIBuilder.cpp | 6 +++--- llvm/lib/IR/DebugInfoODRUniquer.cpp | 6 ++---- 5 files changed, 8 insertions(+), 13 deletions(-) diff --git a/llvm/include/llvm/IR/DebugInfoODRUniquer.h b/llvm/include/llvm/IR/DebugInfoODRUniquer.h index 6533f36cbe6cf09..eb2b33fa9409f74 100644 --- a/llvm/include/llvm/IR/DebugInfoODRUniquer.h +++ b/llvm/include/llvm/IR/DebugInfoODRUniquer.h @@ -85,9 +85,7 @@ class DebugInfoODRUniquer { /// name, type, and template parameters, that has been registered with /// `addSubprogramDecl`, or return nullptr. LLVM_ABI DISubprogram *getODRSubprogramDecl(Metadata *Scope, - StringRef LinkageName, - Metadata *Type, - Metadata *TemplateParams); + StringRef LinkageName); /// Register function declaration DISubprogram, which may be reused in place /// of other ODR-similar DISubprograms (using `getODRSubprogramDecl`). diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index ae67ac1f0b0f67a..1e14d970eb33235 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -6421,7 +6421,7 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) { if (MaybeODRUnique) Result = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( - scope.Val, linkageName.Val->getString(), type.Val, templateParams.Val); + scope.Val, linkageName.Val->getString()); if (!Result) Result = GET_OR_DISTINCT( diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp index 623f36a88202cb8..337689914161f1b 100644 --- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp +++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp @@ -2057,8 +2057,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata( getMDString(Record[3]); if (MaybeODRUnique) SP = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( - getDITypeRefOrNull(Record[1]), getMDString(Record[3])->getString(), - getMDOrNull(Record[6]), getMDOrNull(Record[13 + OffsetB])); + getDITypeRefOrNull(Record[1]), getMDString(Record[3])->getString()); if (!SP) SP = GET_OR_DISTINCT( diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp index 37295ac49479343..8fef496999e5d24 100644 --- a/llvm/lib/IR/DIBuilder.cpp +++ b/llvm/lib/IR/DIBuilder.cpp @@ -1072,7 +1072,7 @@ DISubprogram *DIBuilder::createFunction( // Look up ODR declaration if requested. if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) Node = VMContext.getDebugTypeODRUniquer()->getODRSubprogramDecl( - Context, LinkageName, Ty, TParams.get()); + Context, LinkageName); // Otherwise or if unable, create it. if (!Node) Node = getSubprogram( @@ -1119,8 +1119,8 @@ DISubprogram *DIBuilder::createMethod( DISubprogram *SP = nullptr; // Look up ODR declaration if requested. if (!IsDefinition && VMContext.isODRUniquingDebugTypes()) - SP = VMContext.getDebugTypeODRUniquer()->getODRSubprogramDecl( - Context, LinkageName, Ty, TParams.get()); + SP = VMContext.getDebugTypeODRUniquer()->getODRSubprogramDecl(Context, + LinkageName); // Otherwise or if unable, create it. if (!SP) SP = getSubprogram( diff --git a/llvm/lib/IR/DebugInfoODRUniquer.cpp b/llvm/lib/IR/DebugInfoODRUniquer.cpp index be5163cf997165f..1f545a056e40bd8 100644 --- a/llvm/lib/IR/DebugInfoODRUniquer.cpp +++ b/llvm/lib/IR/DebugInfoODRUniquer.cpp @@ -15,10 +15,8 @@ using namespace llvm; -DISubprogram * -DebugInfoODRUniquer::getODRSubprogramDecl(Metadata *Scope, - StringRef LinkageName, Metadata *Type, - Metadata *TemplateParams) { +DISubprogram *DebugInfoODRUniquer::getODRSubprogramDecl(Metadata *Scope, + StringRef LinkageName) { // Only methods, which have a type scope, are eligable for ODR uniquing. auto *CT = dyn_cast_or_null<DICompositeType>(Scope); if (!CT || !CT->getRawIdentifier()) >From 7540faadd4442fb8a20e30c679cc7d443c2d8d41 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Wed, 30 Sep 2026 17:10:35 +0100 Subject: [PATCH 06/10] explicitly require DIScope --- llvm/include/llvm/IR/DebugInfoODRUniquer.h | 2 +- llvm/lib/AsmParser/LLParser.cpp | 2 +- llvm/lib/Bitcode/Reader/MetadataLoader.cpp | 3 ++- llvm/lib/IR/DebugInfoODRUniquer.cpp | 3 ++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/IR/DebugInfoODRUniquer.h b/llvm/include/llvm/IR/DebugInfoODRUniquer.h index eb2b33fa9409f74..79ae18eec18c925 100644 --- a/llvm/include/llvm/IR/DebugInfoODRUniquer.h +++ b/llvm/include/llvm/IR/DebugInfoODRUniquer.h @@ -84,7 +84,7 @@ class DebugInfoODRUniquer { /// Get an existing DISubprogram declaration with matching scope, linkage /// name, type, and template parameters, that has been registered with /// `addSubprogramDecl`, or return nullptr. - LLVM_ABI DISubprogram *getODRSubprogramDecl(Metadata *Scope, + LLVM_ABI DISubprogram *getODRSubprogramDecl(DIScope *Scope, StringRef LinkageName); /// Register function declaration DISubprogram, which may be reused in place diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 1e14d970eb33235..23900f029f8300c 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -6421,7 +6421,7 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) { if (MaybeODRUnique) Result = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( - scope.Val, linkageName.Val->getString()); + cast<DIScope>(scope.Val), linkageName.Val->getString()); if (!Result) Result = GET_OR_DISTINCT( diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp index 337689914161f1b..b56574b6281e914 100644 --- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp +++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp @@ -2057,7 +2057,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata( getMDString(Record[3]); if (MaybeODRUnique) SP = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( - getDITypeRefOrNull(Record[1]), getMDString(Record[3])->getString()); + cast<DIScope>(getDITypeRefOrNull(Record[1])), + getMDString(Record[3])->getString()); if (!SP) SP = GET_OR_DISTINCT( diff --git a/llvm/lib/IR/DebugInfoODRUniquer.cpp b/llvm/lib/IR/DebugInfoODRUniquer.cpp index 1f545a056e40bd8..c2856393e429bbf 100644 --- a/llvm/lib/IR/DebugInfoODRUniquer.cpp +++ b/llvm/lib/IR/DebugInfoODRUniquer.cpp @@ -11,11 +11,12 @@ //===----------------------------------------------------------------------===// #include "llvm/IR/DebugInfoODRUniquer.h" +#include "llvm/ExecutionEngine/JITLink/JITLink.h" #include "llvm/IR/DebugInfoMetadata.h" using namespace llvm; -DISubprogram *DebugInfoODRUniquer::getODRSubprogramDecl(Metadata *Scope, +DISubprogram *DebugInfoODRUniquer::getODRSubprogramDecl(DIScope *Scope, StringRef LinkageName) { // Only methods, which have a type scope, are eligable for ODR uniquing. auto *CT = dyn_cast_or_null<DICompositeType>(Scope); >From 86cbd3410d334589c456ade5971694b1f4079a21 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Wed, 30 Sep 2026 20:28:27 +0100 Subject: [PATCH 07/10] Handle scope fwd-decls using temporary SPs --- llvm/include/llvm/IR/DebugInfoODRUniquer.h | 17 +++ llvm/lib/AsmParser/LLParser.cpp | 26 ++++- llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 5 +- llvm/lib/Bitcode/Reader/MetadataLoader.cpp | 126 ++++++++++++++------- llvm/lib/Bitcode/Reader/MetadataLoader.h | 3 +- llvm/lib/IR/DebugInfoODRUniquer.cpp | 33 +++++- 6 files changed, 160 insertions(+), 50 deletions(-) diff --git a/llvm/include/llvm/IR/DebugInfoODRUniquer.h b/llvm/include/llvm/IR/DebugInfoODRUniquer.h index 79ae18eec18c925..4dcfd3e355072f7 100644 --- a/llvm/include/llvm/IR/DebugInfoODRUniquer.h +++ b/llvm/include/llvm/IR/DebugInfoODRUniquer.h @@ -77,7 +77,16 @@ class DebugInfoODRUniquer { /// full structural equality. DenseSet<DISubprogram *, DISubprogramODRInfo> FnDecls; + /// Function declarations that cannot be ODR-uniqued yet due to a temporary + /// scope operand. + SmallVector<TempDISubprogram> PendingFnDecls; + public: + ~DebugInfoODRUniquer() { + assert(PendingFnDecls.empty() && + "Unexpected unresolved maybe-ODR-unqiue DISubprograms"); + } + // FIXME: Improve the interface for types. DenseMap<const MDString *, DICompositeType *> DITypeMap; @@ -90,6 +99,14 @@ class DebugInfoODRUniquer { /// Register function declaration DISubprogram, which may be reused in place /// of other ODR-similar DISubprograms (using `getODRSubprogramDecl`). LLVM_ABI void addSubprogramDecl(DISubprogram *SP); + + /// Add a DISubprogram that has a forward declared scope to evaluate later, + /// once finalizeUnresolvedSubprogramDecls is called. + LLVM_ABI void addUnresolvedODRSubprogramDecl(TempDISubprogram SP); + + /// Replace temporaries added with `addUnresolvedODRSubprogramDecl` by + /// attempting to ODR-unique them else making them permanent. + LLVM_ABI void finalizeUnresolvedSubprogramDecls(); }; } // namespace llvm diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 23900f029f8300c..0c0077c447cd974 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -530,6 +530,9 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) { "use of undefined value '@" + Twine(ForwardRefValIDs.begin()->first) + "'"); + if (Context.isODRUniquingDebugTypes()) + Context.getDebugTypeODRUniquer()->finalizeUnresolvedSubprogramDecls(); + // Resolve metadata cycles. for (auto &N : NumberedMetadata) { if (N.second && !N.second->isResolved()) @@ -6419,9 +6422,26 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) { !(SPFlags & DISubprogram::SPFlagDefinition) && linkageName.Val; - if (MaybeODRUnique) - Result = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( - cast<DIScope>(scope.Val), linkageName.Val->getString()); + if (MaybeODRUnique) { + if (DIScope *Scope = dyn_cast<DIScope>(scope.Val)) { + Result = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( + Scope, linkageName.Val->getString()); + } else { + // The scope is a temporary forward reference, meaning we can't perform + // ODR-uniquing yet. In order to perform ODR uniquing later the SP must + // be replacable, so create a temporary one. + TempDISubprogram Tmp = DISubprogram::getTemporary( + Context, scope.Val, name.Val, linkageName.Val, file.Val, line.Val, + type.Val, scopeLine.Val, containingType.Val, virtualIndex.Val, + thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val, + declaration.Val, retainedNodes.Val, thrownTypes.Val, annotations.Val, + targetFuncName.Val, keyInstructions.Val); + Result = Tmp.get(); + Context.getDebugTypeODRUniquer()->addUnresolvedODRSubprogramDecl( + std::move(Tmp)); + return false; + } + } if (!Result) Result = GET_OR_DISTINCT( diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index d75e85b0b0f9e1c..fbeaeac3c1b921f 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -59,6 +59,7 @@ #include "llvm/IR/ModuleSummaryIndex.h" #include "llvm/IR/Operator.h" #include "llvm/IR/ProfDataUtils.h" +#include "llvm/IR/TrackingMDRef.h" #include "llvm/IR/Type.h" #include "llvm/IR/Value.h" #include "llvm/IR/Verifier.h" @@ -7195,8 +7196,8 @@ Error BitcodeReader::materialize(GlobalValue *GV) { stripDebugInfo(*F); // Finish fn->subprogram upgrade for materialized functions. - if (DISubprogram *SP = MDLoader->lookupSubprogramForFunction(F)) - F->setSubprogram(SP); + if (TrackingMDNodeRef SP = MDLoader->lookupSubprogramForFunction(F)) + F->setSubprogram(cast<DISubprogram>(SP)); // Check if the TBAA Metadata are valid, otherwise we will need to strip them. if (!MDLoader->isStrippingTBAA()) { diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp index b56574b6281e914..a9a01c02d9fc840 100644 --- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp +++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp @@ -449,7 +449,7 @@ class MetadataLoader::MetadataLoaderImpl { /// Functions that need to be matched with subprograms when upgrading old /// metadata. - SmallDenseMap<Function *, DISubprogram *, 16> FunctionsWithSPs; + SmallDenseMap<Function *, TrackingMDNodeRef, 16> FunctionsWithSPs; /// retainedNodes of these subprograms should be cleaned up from incorrectly /// scoped local types. @@ -793,7 +793,7 @@ class MetadataLoader::MetadataLoaderImpl { return MetadataList.getMetadataFwdRef(ID); } - DISubprogram *lookupSubprogramForFunction(Function *F) { + TrackingMDNodeRef lookupSubprogramForFunction(Function *F) { return FunctionsWithSPs.lookup(F); } @@ -1260,6 +1260,10 @@ void MetadataLoader::MetadataLoaderImpl::resolveForwardRefsAndPlaceholders( while (MetadataList.hasFwdRefs()) lazyLoadOneMetadata(MetadataList.getNextFwdRef(), Placeholders); } + + if (Context.isODRUniquingDebugTypes()) + Context.getDebugTypeODRUniquer()->finalizeUnresolvedSubprogramDecls(); + // At this point we don't have any forward reference remaining, or temporary // that haven't been loaded. We can safely drop RAUW support and mark cycles // as resolved. @@ -2055,46 +2059,82 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata( bool MaybeODRUnique = Context.isODRUniquingDebugTypes() && !IsDistinct && !(SPFlags & DISubprogram::SPFlagDefinition) && getMDString(Record[3]); - if (MaybeODRUnique) - SP = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( - cast<DIScope>(getDITypeRefOrNull(Record[1])), - getMDString(Record[3])->getString()); - - if (!SP) - SP = GET_OR_DISTINCT( - DISubprogram, - (Context, - getDITypeRefOrNull(Record[1]), // scope - getMDString(Record[2]), // name - getMDString(Record[3]), // linkageName - getMDOrNull(Record[4]), // file - Record[5], // line - getMDOrNull(Record[6]), // type - Record[7 + OffsetA], // scopeLine - getDITypeRefOrNull(Record[8 + OffsetA]), // containingType - Record[10 + OffsetA], // virtualIndex - HasThisAdj ? Record[16 + OffsetB] : 0, // thisAdjustment - Flags, // flags - SPFlags, // SPFlags - HasUnit ? CUorFn : nullptr, // unit - getMDOrNull(Record[13 + OffsetB]), // templateParams - getMDOrNull(Record[14 + OffsetB]), // declaration - getMDOrNull(Record[15 + OffsetB]), // retainedNodes - HasThrownTypes ? getMDOrNull(Record[17 + OffsetB]) - : nullptr, // thrownTypes - HasAnnotations ? getMDOrNull(Record[18 + OffsetB]) - : nullptr, // annotations - HasTargetFuncName ? getMDString(Record[19 + OffsetB]) - : nullptr, // targetFuncName - UsesKeyInstructions)); - MetadataList.assignValue(SP, NextMetadataNo); - NextMetadataNo++; - if (MaybeODRUnique) - Context.getDebugTypeODRUniquer()->addSubprogramDecl(SP); + if (MaybeODRUnique && !isa<DIScope>(getDITypeRefOrNull(Record[1]))) { + // The scope is a temporary forward reference, meaning we can't perform + // ODR-uniquing yet. In order to perform ODR uniquing later the SP must + // be replacable, so create a temporary one. + TempDISubprogram Tmp = DISubprogram::getTemporary( + Context, + getDITypeRefOrNull(Record[1]), // scope + getMDString(Record[2]), // name + getMDString(Record[3]), // linkageName + getMDOrNull(Record[4]), // file + Record[5], // line + getMDOrNull(Record[6]), // type + Record[7 + OffsetA], // scopeLine + getDITypeRefOrNull(Record[8 + OffsetA]), // containingType + Record[10 + OffsetA], // virtualIndex + HasThisAdj ? Record[16 + OffsetB] : 0, // thisAdjustment + Flags, // flags + SPFlags, // SPFlags + HasUnit ? CUorFn : nullptr, // unit + getMDOrNull(Record[13 + OffsetB]), // templateParams + getMDOrNull(Record[14 + OffsetB]), // declaration + getMDOrNull(Record[15 + OffsetB]), // retainedNodes + HasThrownTypes ? getMDOrNull(Record[17 + OffsetB]) + : nullptr, // thrownTypes + HasAnnotations ? getMDOrNull(Record[18 + OffsetB]) + : nullptr, // annotations + HasTargetFuncName ? getMDString(Record[19 + OffsetB]) + : nullptr, // targetFuncName + UsesKeyInstructions); + Context.getDebugTypeODRUniquer()->addUnresolvedODRSubprogramDecl( + std::move(Tmp)); + SP = Tmp.get(); + } else { + + if (MaybeODRUnique) + SP = Context.getDebugTypeODRUniquer()->getODRSubprogramDecl( + cast<DIScope>(getDITypeRefOrNull(Record[1])), + getMDString(Record[3])->getString()); + + if (!SP) + SP = GET_OR_DISTINCT( + DISubprogram, + (Context, + getDITypeRefOrNull(Record[1]), // scope + getMDString(Record[2]), // name + getMDString(Record[3]), // linkageName + getMDOrNull(Record[4]), // file + Record[5], // line + getMDOrNull(Record[6]), // type + Record[7 + OffsetA], // scopeLine + getDITypeRefOrNull(Record[8 + OffsetA]), // containingType + Record[10 + OffsetA], // virtualIndex + HasThisAdj ? Record[16 + OffsetB] : 0, // thisAdjustment + Flags, // flags + SPFlags, // SPFlags + HasUnit ? CUorFn : nullptr, // unit + getMDOrNull(Record[13 + OffsetB]), // templateParams + getMDOrNull(Record[14 + OffsetB]), // declaration + getMDOrNull(Record[15 + OffsetB]), // retainedNodes + HasThrownTypes ? getMDOrNull(Record[17 + OffsetB]) + : nullptr, // thrownTypes + HasAnnotations ? getMDOrNull(Record[18 + OffsetB]) + : nullptr, // annotations + HasTargetFuncName ? getMDString(Record[19 + OffsetB]) + : nullptr, // targetFuncName + UsesKeyInstructions)); + MetadataList.assignValue(SP, NextMetadataNo); + NextMetadataNo++; + + if (MaybeODRUnique) + Context.getDebugTypeODRUniquer()->addSubprogramDecl(SP); - if (IsDistinct) - NewDistinctSPs.push_back(SP); + if (IsDistinct) + NewDistinctSPs.push_back(SP); + } // Upgrade sp->function mapping to function->sp mapping. if (HasFn) { @@ -2102,8 +2142,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata( if (auto *F = dyn_cast<Function>(CMD->getValue())) { if (F->isMaterializable()) // Defer until materialized; unmaterialized functions may not have - // metadata. - FunctionsWithSPs[F] = SP; + // metadata. SP may be temporary, so wrap it in TrackingMDNodeRef. + FunctionsWithSPs[F] = TrackingMDNodeRef(SP); else if (!F->empty()) F->setSubprogram(SP); } @@ -2755,7 +2795,7 @@ Metadata *MetadataLoader::getMetadataFwdRefOrLoad(unsigned Idx) { return Pimpl->getMetadataFwdRefOrLoad(Idx); } -DISubprogram *MetadataLoader::lookupSubprogramForFunction(Function *F) { +TrackingMDNodeRef MetadataLoader::lookupSubprogramForFunction(Function *F) { return Pimpl->lookupSubprogramForFunction(F); } diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.h b/llvm/lib/Bitcode/Reader/MetadataLoader.h index bab855ca6359107..c61acb8396aa119 100644 --- a/llvm/lib/Bitcode/Reader/MetadataLoader.h +++ b/llvm/lib/Bitcode/Reader/MetadataLoader.h @@ -13,6 +13,7 @@ #ifndef LLVM_LIB_BITCODE_READER_METADATALOADER_H #define LLVM_LIB_BITCODE_READER_METADATALOADER_H +#include "llvm/IR/TrackingMDRef.h" #include "llvm/Support/Error.h" #include <functional> @@ -78,7 +79,7 @@ class MetadataLoader { Metadata *getMetadataFwdRefOrLoad(unsigned Idx); /// Return the DISubprogram metadata for a Function if any, null otherwise. - DISubprogram *lookupSubprogramForFunction(Function *F); + TrackingMDNodeRef lookupSubprogramForFunction(Function *F); /// Parse a `METADATA_ATTACHMENT` block for a function. Error parseMetadataAttachment(Function &F, diff --git a/llvm/lib/IR/DebugInfoODRUniquer.cpp b/llvm/lib/IR/DebugInfoODRUniquer.cpp index c2856393e429bbf..cc20e5bb7a7d122 100644 --- a/llvm/lib/IR/DebugInfoODRUniquer.cpp +++ b/llvm/lib/IR/DebugInfoODRUniquer.cpp @@ -11,7 +11,6 @@ //===----------------------------------------------------------------------===// #include "llvm/IR/DebugInfoODRUniquer.h" -#include "llvm/ExecutionEngine/JITLink/JITLink.h" #include "llvm/IR/DebugInfoMetadata.h" using namespace llvm; @@ -35,6 +34,8 @@ DISubprogram *DebugInfoODRUniquer::getODRSubprogramDecl(DIScope *Scope, void DebugInfoODRUniquer::addSubprogramDecl(DISubprogram *SP) { assert(!SP->isDefinition() && "only expect declarations DISubprogram ODR uniquing"); + assert(!SP->isDistinct() && "expect declarations to be uniqued"); + if (SP->getLinkageName().empty()) return; // Only methods, which have a type scope, are eligable for ODR uniquing. @@ -44,3 +45,33 @@ void DebugInfoODRUniquer::addSubprogramDecl(DISubprogram *SP) { FnDecls.insert(SP); } + +void DebugInfoODRUniquer::addUnresolvedODRSubprogramDecl(TempDISubprogram SP) { + assert(!SP->isDefinition() && "expected declarations only"); + if (SP->getLinkageName().empty()) + return; + PendingFnDecls.push_back(std::move(SP)); +} + +void DebugInfoODRUniquer::finalizeUnresolvedSubprogramDecls() { + while (!PendingFnDecls.empty()) { + TempDISubprogram SP = PendingFnDecls.pop_back_val(); + if (MDNode *Scope = dyn_cast_or_null<MDNode>(SP->getRawScope())) { + assert(!SP->getLinkageName().empty() && "expcted linkage name"); + assert(!Scope->isTemporary() && + "expected temporary scope to be replaced"); + if (DISubprogram *Existing = getODRSubprogramDecl(cast<DIScope>(Scope), + SP->getLinkageName())) { + SP->replaceAllUsesWith(Existing); + continue; + } + } + + // Else, keep this one for good. All of this machinery is operating on + // declarations, which shouldn't be distinct. Save it in our map in case + // other instances need to be ODR-uniqued to it. + auto *NewSP = DISubprogram::replaceWithPermanent(std::move(SP)); + assert(NewSP->isResolved() && "expected SP to be resolved"); + addSubprogramDecl(NewSP); + } +} \ No newline at end of file >From 613ee7175a002f4087e5afa03d12cbf942f000da Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Thu, 1 Oct 2026 09:56:06 +0100 Subject: [PATCH 08/10] test, doesn't pass due to disagreement on order of metadata --- .../test/Assembler/dicompositetype-members.ll | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/llvm/test/Assembler/dicompositetype-members.ll b/llvm/test/Assembler/dicompositetype-members.ll index 288358abe8dded8..93ad3057046c014 100644 --- a/llvm/test/Assembler/dicompositetype-members.ll +++ b/llvm/test/Assembler/dicompositetype-members.ll @@ -1,8 +1,11 @@ ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s ; RUN: opt < %s -S | FileCheck %s +; RUN: opt < %s -disable-debug-info-type-map | opt -S | FileCheck %s ; RUN: verify-uselistorder %s -;; This test also runs opt to check opt applies the same odr type debug uniquing. +;; This test also runs opt to check opt applies the same odr type debug uniquing, +;; and runs it with -disable-debug-info-type-map to check that the subsequent +;; bitcode parsing run agrees with the output. ; Anchor the order of the nodes. !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17} @@ -15,19 +18,21 @@ !1 = !DIFile(filename: "path/to/file", directory: "/path/to/dir") !2 = !DIFile(filename: "path/to/other", directory: "/path/to/dir") -; Define an identified type with fields and functions. -; CHECK-NEXT: !3 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid",{{.*}}, identifier: "uuid") -; CHECK-NEXT: !4 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !1 -; CHECK-NEXT: !5 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !3, file: !1 -; CHECK-NEXT: !6 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !1, type: !7, spFlags: 0) -; CHECK-NEXT: !7 = !DISubroutineType(types: !8) -; CHECK-NEXT: !8 = !{null} -; CHECK-NEXT: !9 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !3, file: !1, type: !7, spFlags: 0) -!3 = !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid", file: !1, line: 2, size: 64, align: 32, identifier: "uuid") -!4 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!5 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !3, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!6 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !1, isDefinition: false, type: !18) -!7 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !3, file: !1, isDefinition: false, type: !18) +; Define an identified type with fields and functions. DISubprograms come +; before DICompositeType scope intentionally to check forward refs work +; with the debug ODR type uniquing infra. +; CHECK-NEXT: !3 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !4, file: !2, type: !5, spFlags: 0) +; CHECK-NEXT: !4 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid",{{.*}}, identifier: "uuid") +; CHECK-NEXT: !5 = !DISubroutineType(types: !6) +; CHECK-NEXT: !6 = !{null} +; CHECK-NEXT: !7 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !4, file: !1, type: !5, spFlags: 0) +; CHECK-NEXT: !8 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !4, file: !1 +; CHECK-NEXT: !9 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !4, file: !1 +!3 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !5, file: !1, isDefinition: false, type: !18) +!4 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !5, file: !1, isDefinition: false, type: !18) +!5 = !DICompositeType(tag: DW_TAG_structure_type, name: "has-uuid", file: !1, line: 2, size: 64, align: 32, identifier: "uuid") +!6 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !5, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!7 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !5, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) !18 = !DISubroutineType(types: !19) !19 = !{null} @@ -35,8 +40,8 @@ ; CHECK-NEXT: !10 = !DICompositeType(tag: DW_TAG_structure_type, name: "no-uuid", file: !1 ; CHECK-NEXT: !11 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !10, file: !1 ; CHECK-NEXT: !12 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !10, file: !1 -; CHECK-NEXT: !13 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !1, type: !7, spFlags: 0) -; CHECK-NEXT: !14 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !10, file: !1, type: !7, spFlags: 0) +; CHECK-NEXT: !13 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !1, type: !5, spFlags: 0) +; CHECK-NEXT: !14 = !DISubprogram(name: "foo", linkageName: "foo2", scope: !10, file: !1, type: !5, spFlags: 0) !8 = !DICompositeType(tag: DW_TAG_structure_type, name: "no-uuid", file: !1, line: 2, size: 64, align: 32) !9 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !8, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) !10 = !DIDerivedType(tag: DW_TAG_member, name: "field2", scope: !8, file: !1, line: 4, baseType: !0, size: 32, align: 32, offset: 32) @@ -46,16 +51,16 @@ ; Add duplicate fields and members of "no-uuid" in a different file. These ; should stick around, since "no-uuid" does not have an "identifier:" field. ; CHECK-NEXT: !15 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !10, file: !2, -; CHECK-NEXT: !16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !2, type: !7, spFlags: 0) +; CHECK-NEXT: !16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !10, file: !2, type: !5, spFlags: 0) !13 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !8, file: !2, line: 4, baseType: !0, size: 32, align: 32, offset: 32) !14 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !8, file: !2, isDefinition: false, type: !18) ; Add duplicate fields and members of "has-uuid" in a different file. These ; should be merged. -!15 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !3, file: !2, line: 4, baseType: !0, size: 32, align: 32, offset: 32) -!16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !3, file: !2, isDefinition: false, type: !18) +!15 = !DIDerivedType(tag: DW_TAG_member, name: "field1", scope: !5, file: !2, line: 4, baseType: !0, size: 32, align: 32, offset: 32) +!16 = !DISubprogram(name: "foo", linkageName: "foo1", scope: !5, file: !2, isDefinition: false, type: !18) -; CHECK-NEXT: !17 = !{!4, !6} +; CHECK-NEXT: !17 = !{!8, !3} ; CHECK-NOT: !DIDerivedType ; CHECK-NOT: !DISubprogram !17 = !{!15, !16} >From 12adcbe095508b5945ecfbf748165086aa5fe75e Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Thu, 1 Oct 2026 10:47:58 +0100 Subject: [PATCH 09/10] nfc: fix isEqual --- llvm/include/llvm/IR/DebugInfoODRUniquer.h | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/IR/DebugInfoODRUniquer.h b/llvm/include/llvm/IR/DebugInfoODRUniquer.h index 4dcfd3e355072f7..ec5f47b8ae5a413 100644 --- a/llvm/include/llvm/IR/DebugInfoODRUniquer.h +++ b/llvm/include/llvm/IR/DebugInfoODRUniquer.h @@ -35,18 +35,12 @@ struct DISubprogramODRKey { } static bool isEqual(const DISubprogramODRKey &LHS, const DISubprogram *RHS) { - if (LHS.LinkageName.empty() || LHS.ScopeIdentifier.empty()) - return false; - - auto *CT = dyn_cast_or_null<DICompositeType>(RHS->getRawScope()); - if (!CT || !CT->getRawIdentifier()) - return false; - - if (!RHS->getRawLinkageName()) - return false; + StringRef RHSIdentifier; + if (auto *CT = dyn_cast_or_null<DICompositeType>(RHS->getRawScope())) + RHSIdentifier = CT->getIdentifier(); return LHS.LinkageName == RHS->getLinkageName() && - LHS.ScopeIdentifier == CT->getIdentifier(); + LHS.ScopeIdentifier == RHSIdentifier; } static bool isEqual(const DISubprogram *LHS, const DISubprogram *RHS) { >From c3e3b9b42cbde51a4a188176beb6156ddb5a7e7f Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams <[email protected]> Date: Thu, 1 Oct 2026 11:54:17 +0100 Subject: [PATCH 10/10] add comment to unittest --- .../Transforms/Utils/CloningTest.cpp | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/llvm/unittests/Transforms/Utils/CloningTest.cpp b/llvm/unittests/Transforms/Utils/CloningTest.cpp index 1824a79ee655297..85343c207517a24 100644 --- a/llvm/unittests/Transforms/Utils/CloningTest.cpp +++ b/llvm/unittests/Transforms/Utils/CloningTest.cpp @@ -1442,7 +1442,8 @@ TEST_F(CloneModule, GlobalWithBlockAddressesInitializer) { TEST_F(CloneModule, ODRUniqueTypeMethodVerifier) { // Test that the following IR, reduced from a self-host clang-22 compile, - // doesn't trip the verifier after cloning. + // doesn't trip the verifier after cloning. See the IR body for an additional + // explanatory comment. // // The focus of the test is "operator!=" which is a DISubprogram that is: // * a declaration (therefore not distinct), @@ -1496,6 +1497,30 @@ TEST_F(CloneModule, ODRUniqueTypeMethodVerifier) { !1 = !DIFile(filename: "reduced.cpp", directory: "/") !2 = !{} !3 = !{i32 2, !"Debug Info Version", i32 3} +;; +;; operator!= (!4) has a `scope` (!5) that is an ODR type, meaning ODR-uniquing +;; takes place for the DISubprogram. Its `type` (!6) includes a distinct +;; DICompositeType (!8). +;; +;; The old debug ODR-uniquing infrastructure was implemented such that +;; DISubprograms that meet certain conditions (such as !4) are uniqued is a +;; subset of fields are equal (`scope`, `linkageName`, `templateParams`) rather +;; than all fields being equal which is how uniquing usually works. +;; +;; CloneModule clones and remaps the operands. With the old scheme described +;; above, the two cloned DISubprogram are collapsed down to one because +;; the scope, linkageName and templateParams are equal between the two +;; instances (importantly the scope is not distinct). +;; +;; The distinct DICompositeType (!8) thus becomes reachable from the new +;; module, which is an error. A symptom of this is that the original module's +;; DICompileUnit is reachable from there too, and the verifier spots this +;; because it isn't in the module's llvm.dbg.cu list. +;; +;; Obviously a cloned module shouldn't produce invalid IR and to achieve this +;; debug ODR uniquing is now only enabled at strategic times rather than being +;; always-on. +;; !4 = !DISubprogram(name: "operator!=", linkageName: "_ZN6BaseItI2ItIZN2ar3lolEvEUlvE_EEneES3_", scope: !5, file: !1, line: 4, type: !6, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized) !5 = !DICompositeType(tag: DW_TAG_class_type, name: "BaseIt<It<(lambda)> >", file: !1, line: 1, size: 8, flags: DIFlagFwdDecl | DIFlagNonTrivial, identifier: "_ZTS6BaseItI2ItIZN2ar3lolEvEUlvE_EE") !6 = distinct !DISubroutineType(types: !7) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
