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

@@ -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),
+      : DIScope(C, ID, Storage, Tag, Ops) {
-        SizeInBits(SizeInBits), AlignInBits(AlignInBits),
+    init(Line, SizeInBits, AlignInBits, OffsetInBits, Flags);
-        OffsetInBits(OffsetInBits) {}
+  }
   ~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());

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
+  // If this has an identifier try to build an ODR type.
-  // type map in the context.
+  if (identifier.Val)
-  if (!(flags.Val & DINode::FlagFwdDecl) && identifier.Val)
+    if (auto *CT = DICompositeType::buildODRType(
-    if (auto *CT = DICompositeType::getODRType(
             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,

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)
+      if (Identifier)
-        CT = DICompositeType::getODRType(
+        CT = DICompositeType::buildODRType(
             Context, *Identifier, Tag, Name, File, Line, Scope, BaseType,
             SizeInBits, AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang,
             VTableHolder, TemplateParams);

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,

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)

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}
+; CHECK:   !named = !{!0, !1, !2, !3, !0, !1, !2, !3}
-; NOMAP:   !named = !{!0, !1, !0, !2}
+; NOMAP:   !named = !{!0, !1, !2, !3, !0, !4, !5, !6}
-!named = !{!0, !1}
+!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)

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