From 9738602869dda75509a0f3782cd6639f9e3927c4 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Tue, 19 Apr 2016 18:00:19 +0000 Subject: [PATCH] IR: Enable debug info type ODR uniquing for forward decls Add a new method, DICompositeType::buildODRType, that will create or mutate the DICompositeType for a given ODR identifier, and use it in LLParser and BitcodeReader instead of DICompositeType::getODRType. The logic is as follows: - If there's no node, create one with the given arguments. - Else, if the current node is a forward declaration and the new arguments would create a definition, mutate the node to match the new arguments. - Else, return the old node. This adds a missing feature supported by the current DITypeIdentifierMap (which I'm slowly making redudant). The only remaining difference is that the DITypeIdentifierMap has a "the-last-one-wins" rule, whereas DICompositeType::buildODRType has a "the-first-one-wins" rule. For now I'm leaving behind DICompositeType::getODRType since it has obvious, low-level semantics that are convenient for unit testing. llvm-svn: 266786 --- llvm/include/llvm/IR/DebugInfoMetadata.h | 53 ++++++++++- llvm/lib/AsmParser/LLParser.cpp | 7 +- llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 4 +- llvm/lib/IR/DebugInfoMetadata.cpp | 35 +++++++ .../Linker/Inputs/dicompositetype-unique.ll | 4 +- llvm/test/Linker/dicompositetype-unique.ll | 31 ++++++- .../unittests/IR/DebugTypeODRUniquingTest.cpp | 91 +++++++++++++++++++ 7 files changed, 212 insertions(+), 13 deletions(-) diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h index 61bb1334196b..355f892d4ec7 100644 --- a/llvm/include/llvm/IR/DebugInfoMetadata.h +++ b/llvm/include/llvm/IR/DebugInfoMetadata.h @@ -175,6 +175,9 @@ protected: return MDString::get(Context, S); } + /// Allow subclasses to mutate the tag. + void setTag(unsigned Tag) { SubclassData16 = Tag; } + public: unsigned getTag() const { return SubclassData16; } @@ -530,11 +533,28 @@ protected: DIType(LLVMContext &C, unsigned ID, StorageType Storage, unsigned Tag, unsigned Line, uint64_t SizeInBits, uint64_t AlignInBits, uint64_t OffsetInBits, unsigned Flags, ArrayRef Ops) - : DIScope(C, ID, Storage, Tag, Ops), Line(Line), Flags(Flags), - SizeInBits(SizeInBits), AlignInBits(AlignInBits), - OffsetInBits(OffsetInBits) {} + : DIScope(C, ID, Storage, Tag, Ops) { + init(Line, SizeInBits, AlignInBits, OffsetInBits, Flags); + } ~DIType() = default; + void init(unsigned Line, uint64_t SizeInBits, uint64_t AlignInBits, + uint64_t OffsetInBits, unsigned Flags) { + this->Line = Line; + this->Flags = Flags; + this->SizeInBits = SizeInBits; + this->AlignInBits = AlignInBits; + this->OffsetInBits = OffsetInBits; + } + + /// Change fields in place. + void mutate(unsigned Tag, unsigned Line, uint64_t SizeInBits, + uint64_t AlignInBits, uint64_t OffsetInBits, unsigned Flags) { + assert(isDistinct() && "Only distinct nodes can mutate"); + setTag(Tag); + init(Line, SizeInBits, AlignInBits, OffsetInBits, Flags); + } + public: TempDIType clone() const { return TempDIType(cast(MDNode::clone().release())); @@ -770,6 +790,16 @@ class DICompositeType : public DIType { RuntimeLang(RuntimeLang) {} ~DICompositeType() = default; + /// Change fields in place. + void mutate(unsigned Tag, unsigned Line, unsigned RuntimeLang, + uint64_t SizeInBits, uint64_t AlignInBits, uint64_t OffsetInBits, + unsigned Flags) { + assert(isDistinct() && "Only distinct nodes can mutate"); + assert(getRawIdentifier() && "Only ODR-uniqued nodes should mutate"); + this->RuntimeLang = RuntimeLang; + DIType::mutate(Tag, Line, SizeInBits, AlignInBits, OffsetInBits, Flags); + } + static DICompositeType * getImpl(LLVMContext &Context, unsigned Tag, StringRef Name, Metadata *File, unsigned Line, DIScopeRef Scope, DITypeRef BaseType, @@ -842,6 +872,23 @@ public: static DICompositeType *getODRTypeIfExists(LLVMContext &Context, MDString &Identifier); + /// Build a DICompositeType with the given ODR identifier. + /// + /// Looks up the mapped DICompositeType for the given ODR \c Identifier. If + /// it doesn't exist, creates a new one. If it does exist and \a + /// isForwardDecl(), and the new arguments would be a definition, mutates the + /// the type in place. In either case, returns the type. + /// + /// If not \a LLVMContext::isODRUniquingDebugTypes(), this function returns + /// nullptr. + static DICompositeType * + buildODRType(LLVMContext &Context, MDString &Identifier, unsigned Tag, + MDString *Name, Metadata *File, unsigned Line, Metadata *Scope, + Metadata *BaseType, uint64_t SizeInBits, uint64_t AlignInBits, + uint64_t OffsetInBits, unsigned Flags, Metadata *Elements, + unsigned RuntimeLang, Metadata *VTableHolder, + Metadata *TemplateParams); + DITypeRef getBaseType() const { return DITypeRef(getRawBaseType()); } DINodeArray getElements() const { return cast_or_null(getRawElements()); diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index fb27ab5353fc..d9b7f3efe94c 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -3839,10 +3839,9 @@ bool LLParser::ParseDICompositeType(MDNode *&Result, bool IsDistinct) { PARSE_MD_FIELDS(); #undef VISIT_MD_FIELDS - // If this isn't a forward declaration and it has a UUID, check for it in the - // type map in the context. - if (!(flags.Val & DINode::FlagFwdDecl) && identifier.Val) - if (auto *CT = DICompositeType::getODRType( + // If this has an identifier try to build an ODR type. + if (identifier.Val) + if (auto *CT = DICompositeType::buildODRType( Context, *identifier.Val, tag.Val, name.Val, file.Val, line.Val, scope.Val, baseType.Val, size.Val, align.Val, offset.Val, flags.Val, elements.Val, runtimeLang.Val, vtableHolder.Val, diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index dba0fd417552..9c65eecad81a 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -2207,8 +2207,8 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { Metadata *TemplateParams = getMDOrNull(Record[14]); auto *Identifier = getMDString(Record[15]); DICompositeType *CT = nullptr; - if (!(Flags & DINode::FlagFwdDecl) && Identifier) - CT = DICompositeType::getODRType( + if (Identifier) + CT = DICompositeType::buildODRType( Context, *Identifier, Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang, VTableHolder, TemplateParams); diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp index 37986f73ab1a..c2ecd4e5fc81 100644 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -255,6 +255,7 @@ DICompositeType *DICompositeType::getImpl( bool ShouldCreate) { assert(isCanonical(Name) && "Expected canonical MDString"); + // Keep this in sync with buildODRType. DEFINE_GETIMPL_LOOKUP( DICompositeType, (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang, @@ -266,6 +267,40 @@ DICompositeType *DICompositeType::getImpl( Ops); } +DICompositeType *DICompositeType::buildODRType( + LLVMContext &Context, MDString &Identifier, unsigned Tag, MDString *Name, + Metadata *File, unsigned Line, Metadata *Scope, Metadata *BaseType, + uint64_t SizeInBits, uint64_t AlignInBits, uint64_t OffsetInBits, + unsigned Flags, Metadata *Elements, unsigned RuntimeLang, + Metadata *VTableHolder, Metadata *TemplateParams) { + assert(!Identifier.getString().empty() && "Expected valid identifier"); + if (!Context.isODRUniquingDebugTypes()) + return nullptr; + auto *&CT = (*Context.pImpl->DITypeMap)[&Identifier]; + if (!CT) + return CT = DICompositeType::getDistinct( + Context, Tag, Name, File, Line, Scope, BaseType, SizeInBits, + AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang, + VTableHolder, TemplateParams, &Identifier); + + // Only mutate CT if it's a forward declaration and the new operands aren't. + assert(CT->getRawIdentifier() == &Identifier && "Wrong ODR identifier?"); + if (!CT->isForwardDecl() || (Flags & DINode::FlagFwdDecl)) + return CT; + + // Mutate CT in place. Keep this in sync with getImpl. + CT->mutate(Tag, Line, RuntimeLang, SizeInBits, AlignInBits, OffsetInBits, + Flags); + Metadata *Ops[] = {File, Scope, Name, BaseType, + Elements, VTableHolder, TemplateParams, &Identifier}; + assert(std::end(Ops) - std::begin(Ops) == CT->getNumOperands() && + "Mismatched number of operands"); + for (unsigned I = 0, E = CT->getNumOperands(); I != E; ++I) + if (Ops[I] != CT->getOperand(I)) + CT->setOperand(I, Ops[I]); + return CT; +} + DICompositeType *DICompositeType::getODRType( LLVMContext &Context, MDString &Identifier, unsigned Tag, MDString *Name, Metadata *File, unsigned Line, Metadata *Scope, Metadata *BaseType, diff --git a/llvm/test/Linker/Inputs/dicompositetype-unique.ll b/llvm/test/Linker/Inputs/dicompositetype-unique.ll index c2389e9a63ca..e1537b93dfec 100644 --- a/llvm/test/Linker/Inputs/dicompositetype-unique.ll +++ b/llvm/test/Linker/Inputs/dicompositetype-unique.ll @@ -1,4 +1,6 @@ -!named = !{!0, !1} +!named = !{!0, !1, !2, !3} !0 = !DIFile(filename: "abc", directory: "/path/to") !1 = !DICompositeType(tag: DW_TAG_class_type, name: "T2", identifier: "T", file: !0) +!2 = !DICompositeType(tag: DW_TAG_class_type, name: "FwdTDef", identifier: "FwdT", file: !0) +!3 = !DICompositeType(tag: DW_TAG_class_type, flags: DIFlagFwdDecl, name: "BothFwdT2", identifier: "BothFwdT", file: !0) diff --git a/llvm/test/Linker/dicompositetype-unique.ll b/llvm/test/Linker/dicompositetype-unique.ll index 6e4e34bdc2a6..ab1fdaa3616c 100644 --- a/llvm/test/Linker/dicompositetype-unique.ll +++ b/llvm/test/Linker/dicompositetype-unique.ll @@ -17,9 +17,9 @@ ; Check that the type map will unique two DICompositeTypes. -; CHECK: !named = !{!0, !1, !0, !1} -; NOMAP: !named = !{!0, !1, !0, !2} -!named = !{!0, !1} +; CHECK: !named = !{!0, !1, !2, !3, !0, !1, !2, !3} +; NOMAP: !named = !{!0, !1, !2, !3, !0, !4, !5, !6} +!named = !{!0, !1, !2, !3} ; Check both directions. ; CHECK: !1 = distinct !DICompositeType( @@ -27,14 +27,39 @@ ; REVERSE-SAME: name: "T2" ; CHECK-SAME: identifier: "T" ; CHECK-NOT: identifier: "T" +; CHECK: !2 = distinct !DICompositeType( +; CHECK-SAME: name: "FwdTDef" +; CHECK-SAME: identifier: "FwdT" +; CHECK-NOT: identifier: "FwdT" +; CHECK: !3 = distinct !DICompositeType( +; FORWARD-SAME: name: "BothFwdT1" +; REVERSE-SAME: name: "BothFwdT2" +; CHECK-SAME: identifier: "BothFwdT" +; CHECK-NOT: identifier: "BothFwdT" ; These types are different, so we should get both copies when there is no map. ; NOMAP: !1 = !DICompositeType( ; NOMAP-SAME: name: "T1" ; NOMAP-SAME: identifier: "T" ; NOMAP: !2 = !DICompositeType( +; NOMAP-SAME: name: "FwdTFwd" +; NOMAP-SAME: identifier: "FwdT" +; NOMAP: !3 = !DICompositeType( +; NOMAP-SAME: name: "BothFwdT1" +; NOMAP-SAME: identifier: "BothFwdT" +; NOMAP: !4 = !DICompositeType( ; NOMAP-SAME: name: "T2" ; NOMAP-SAME: identifier: "T" ; NOMAP-NOT: identifier: "T" +; NOMAP: !5 = !DICompositeType( +; NOMAP-SAME: name: "FwdTDef" +; NOMAP-SAME: identifier: "FwdT" +; NOMAP-NOT: identifier: "FwdT" +; NOMAP: !6 = !DICompositeType( +; NOMAP-SAME: name: "BothFwdT2" +; NOMAP-SAME: identifier: "BothFwdT" +; NOMAP-NOT: identifier: "BothFwdT" !0 = !DIFile(filename: "abc", directory: "/path/to") !1 = !DICompositeType(tag: DW_TAG_class_type, name: "T1", identifier: "T", file: !0) +!2 = !DICompositeType(tag: DW_TAG_class_type, flags: DIFlagFwdDecl, name: "FwdTFwd", identifier: "FwdT", file: !0) +!3 = !DICompositeType(tag: DW_TAG_class_type, flags: DIFlagFwdDecl, name: "BothFwdT1", identifier: "BothFwdT", file: !0) diff --git a/llvm/unittests/IR/DebugTypeODRUniquingTest.cpp b/llvm/unittests/IR/DebugTypeODRUniquingTest.cpp index b5a379f49bd5..2c899d85d1f3 100644 --- a/llvm/unittests/IR/DebugTypeODRUniquingTest.cpp +++ b/llvm/unittests/IR/DebugTypeODRUniquingTest.cpp @@ -62,4 +62,95 @@ TEST(DebugTypeODRUniquingTest, getODRType) { EXPECT_FALSE(DICompositeType::getODRTypeIfExists(Context, UUID)); } +TEST(DebugTypeODRUniquingTest, buildODRType) { + LLVMContext Context; + Context.enableDebugTypeODRUniquing(); + + // Build an ODR type that's a forward decl. + MDString &UUID = *MDString::get(Context, "Type"); + auto &CT = *DICompositeType::buildODRType( + Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr, 0, nullptr, + nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr, 0, nullptr, nullptr); + EXPECT_EQ(&CT, DICompositeType::getODRTypeIfExists(Context, UUID)); + EXPECT_EQ(dwarf::DW_TAG_class_type, CT.getTag()); + + // Update with another forward decl. This should be a no-op. + EXPECT_EQ(&CT, DICompositeType::buildODRType( + Context, UUID, dwarf::DW_TAG_structure_type, nullptr, nullptr, 0, nullptr, + nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr, 0, nullptr, nullptr)); + EXPECT_EQ(dwarf::DW_TAG_class_type, CT.getTag()); + + // Update with a definition. This time we should see a change. + EXPECT_EQ(&CT, DICompositeType::buildODRType( + Context, UUID, dwarf::DW_TAG_structure_type, nullptr, nullptr, 0, nullptr, + nullptr, 0, 0, 0, 0, nullptr, 0, nullptr, nullptr)); + EXPECT_EQ(dwarf::DW_TAG_structure_type, CT.getTag()); + + // Further updates should be ignored. + EXPECT_EQ(&CT, DICompositeType::buildODRType( + Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr, 0, nullptr, + nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr, 0, nullptr, nullptr)); + EXPECT_EQ(dwarf::DW_TAG_structure_type, CT.getTag()); + EXPECT_EQ(&CT, DICompositeType::buildODRType( + Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr, 0, nullptr, + nullptr, 0, 0, 0, 0, nullptr, 0, nullptr, nullptr)); + EXPECT_EQ(dwarf::DW_TAG_structure_type, CT.getTag()); +} + +TEST(DebugTypeODRUniquingTest, buildODRTypeFields) { + LLVMContext Context; + Context.enableDebugTypeODRUniquing(); + + // Build an ODR type that's a forward decl with no other fields set. + MDString &UUID = *MDString::get(Context, "UUID"); + auto &CT = *DICompositeType::buildODRType( + Context, UUID, 0, nullptr, nullptr, 0, nullptr, nullptr, 0, 0, 0, + DINode::FlagFwdDecl, nullptr, 0, nullptr, nullptr); + +// Create macros for running through all the fields except Identifier and Flags. +#define FOR_EACH_MDFIELD() \ + DO_FOR_FIELD(Name) \ + DO_FOR_FIELD(File) \ + DO_FOR_FIELD(Scope) \ + DO_FOR_FIELD(BaseType) \ + DO_FOR_FIELD(Elements) \ + DO_FOR_FIELD(VTableHolder) \ + DO_FOR_FIELD(TemplateParams) +#define FOR_EACH_INLINEFIELD() \ + DO_FOR_FIELD(Tag) \ + DO_FOR_FIELD(Line) \ + DO_FOR_FIELD(SizeInBits) \ + DO_FOR_FIELD(AlignInBits) \ + DO_FOR_FIELD(OffsetInBits) \ + DO_FOR_FIELD(RuntimeLang) + +// Create all the fields. +#define DO_FOR_FIELD(X) auto *X = MDString::get(Context, #X); + FOR_EACH_MDFIELD(); +#undef DO_FOR_FIELD + unsigned NonZeroInit = 0; +#define DO_FOR_FIELD(X) auto X = ++NonZeroInit; + FOR_EACH_INLINEFIELD(); +#undef DO_FOR_FIELD + + // Replace all the fields with new values that are distinct from each other. + EXPECT_EQ(&CT, + DICompositeType::buildODRType( + Context, UUID, Tag, Name, File, Line, Scope, BaseType, + SizeInBits, AlignInBits, OffsetInBits, DINode::FlagArtificial, + Elements, RuntimeLang, VTableHolder, TemplateParams)); + + // Confirm that all the right fields got updated. +#define DO_FOR_FIELD(X) EXPECT_EQ(X, CT.getRaw##X()); + FOR_EACH_MDFIELD(); +#undef DO_FOR_FIELD +#undef FOR_EACH_MDFIELD +#define DO_FOR_FIELD(X) EXPECT_EQ(X, CT.get##X()); + FOR_EACH_INLINEFIELD(); +#undef DO_FOR_FIELD +#undef FOR_EACH_INLINEFIELD + EXPECT_EQ(DINode::FlagArtificial, CT.getFlags()); + EXPECT_EQ(&UUID, CT.getRawIdentifier()); +} + } // end namespace