[llvm] r266786 - IR: Enable debug info type ODR uniquing for forward decls
Robinson, Paul via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 19 14:17:51 PDT 2016
> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Duncan P. N. Exon Smith via llvm-commits
> Sent: Tuesday, April 19, 2016 11:00 AM
> To: llvm-commits at lists.llvm.org
> Subject: [llvm] r266786 - IR: Enable debug info type ODR uniquing for
> forward decls
>
> Author: dexonsmith
> Date: Tue Apr 19 13:00:19 2016
> New Revision: 266786
>
> URL: http://llvm.org/viewvc/llvm-project?rev=266786&view=rev
> Log:
> 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.
>
> Modified:
> llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
> llvm/trunk/lib/AsmParser/LLParser.cpp
> llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> llvm/trunk/lib/IR/DebugInfoMetadata.cpp
> llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll
> llvm/trunk/test/Linker/dicompositetype-unique.ll
> llvm/trunk/unittests/IR/DebugTypeODRUniquingTest.cpp
>
> Modified: llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/include/llvm/IR/DebugInfoMetadata.h?rev=266786&r1=26678
> 5&r2=266786&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/include/llvm/IR/DebugInfoMetadata.h (original)
> +++ llvm/trunk/include/llvm/IR/DebugInfoMetadata.h Tue Apr 19 13:00:19
> 2016
> @@ -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<Metadata *> 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<DIType>(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<MDTuple>(getRawElements());
>
> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=266786&r1=266785&r2=2667
> 86&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Tue Apr 19 13:00:19 2016
> @@ -3839,10 +3839,9 @@ bool LLParser::ParseDICompositeType(MDNo
> 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,
>
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=266786&r1=2667
> 85&r2=266786&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Tue Apr 19 13:00:19
> 2016
> @@ -2207,8 +2207,8 @@ std::error_code BitcodeReader::parseMeta
> 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);
>
> Modified: llvm/trunk/lib/IR/DebugInfoMetadata.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/IR/DebugInfoMetadata.cpp?rev=266786&r1=266785&r2=26
> 6786&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/lib/IR/DebugInfoMetadata.cpp (original)
> +++ llvm/trunk/lib/IR/DebugInfoMetadata.cpp Tue Apr 19 13:00:19 2016
> @@ -255,6 +255,7 @@ DICompositeType *DICompositeType::getImp
> 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::getImp
> 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;
So, this is where we'd want to detect the case in PR24923?
--paulr
> +
> + // 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,
>
> Modified: llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Linker/Inputs/dicompositetype-
> unique.ll?rev=266786&r1=266785&r2=266786&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll (original)
> +++ llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll Tue Apr 19
> 13:00:19 2016
> @@ -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)
>
> Modified: llvm/trunk/test/Linker/dicompositetype-unique.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Linker/dicompositetype-
> unique.ll?rev=266786&r1=266785&r2=266786&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/test/Linker/dicompositetype-unique.ll (original)
> +++ llvm/trunk/test/Linker/dicompositetype-unique.ll Tue Apr 19 13:00:19
> 2016
> @@ -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)
>
> Modified: llvm/trunk/unittests/IR/DebugTypeODRUniquingTest.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/unittests/IR/DebugTypeODRUniquingTest.cpp?rev=266786&r1
> =266785&r2=266786&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/unittests/IR/DebugTypeODRUniquingTest.cpp (original)
> +++ llvm/trunk/unittests/IR/DebugTypeODRUniquingTest.cpp Tue Apr 19
> 13:00:19 2016
> @@ -62,4 +62,95 @@ TEST(DebugTypeODRUniquingTest, getODRTyp
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list