Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DebugInfo][BPF] Add 'annotations' field for DIBasicType & DISubroutineType #91422

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

eddyz87
Copy link
Contributor

@eddyz87 eddyz87 commented May 8, 2024

Extend DIBasicType and DISubroutineType with additional field annotations, e.g. as below:

  !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed, annotations: !6)
  !6 = !{!7}
  !7 = !{!"btf:type_tag", !"tag1"}

The field would be used by BPF backend to generate DWARF attributes corresponding to btf_type_tag type attributes, e.g.:

  0x00000029:   DW_TAG_base_type
                  DW_AT_name	("int")
                  DW_AT_encoding	(DW_ATE_signed)
                  DW_AT_byte_size	(0x04)

  0x0000002d:     DW_TAG_LLVM_annotation
                    DW_AT_name	("btf:type_tag")
                    DW_AT_const_value	("tag1")

Such DWARF entries would be used to generate BTF definitions by tools like pahole.

Note: similar fields with similar purposes are already present in DIDerivedType and DICompositeType.

Currently "btf_type_tag" attributes are represented in debug information as 'annotations' fields in DIDerivedType with DW_TAG_pointer_type tag. The annotation on a pointer corresponds to pointee having the attributes in the final BTF.

The discussion in thread came to conclusion, that such annotations should apply to the annotated type itself. Hence the necessity to extend DIBasicType & DISubroutineType types with 'annotations' field to represent cases like below:

  int __attribute__((btf_type_tag("foo"))) bar;

This was previously tracked as differential revision: https://reviews.llvm.org/D143966

@eddyz87 eddyz87 changed the title [DebugInfo][BPF] Add 'annotations' field for DIBasicType & DISubrouti… [DebugInfo][BPF] Add 'annotations' field for DIBasicType & DISubroutineType May 8, 2024
@eddyz87
Copy link
Contributor Author

eddyz87 commented May 8, 2024

Hi @dwblaikie, @yonghong-song,

Could you please take a look at this pull request?
This is a part of the old discussion:

But because of the state of the Linux Kernel tooling (the main client for this feature is the kernel) I had to add a hidden switch in the second patch of the series (compared to agreed upon logic).

@dafaust, @jemarch, @4ast, fyi.

@eddyz87 eddyz87 marked this pull request as ready for review May 8, 2024 17:53
@llvmbot
Copy link
Collaborator

llvmbot commented May 8, 2024

@llvm/pr-subscribers-llvm-ir

Author: None (eddyz87)

Changes

Extend DIBasicType and DISubroutineType with additional field annotations, e.g. as below:

  !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed, annotations: !6)
  !6 = !{!7}
  !7 = !{!"btf:type_tag", !"tag1"}

The field would be used by BPF backend to generate DWARF attributes corresponding to btf_type_tag type attributes, e.g.:

  0x00000029:   DW_TAG_base_type
                  DW_AT_name	("int")
                  DW_AT_encoding	(DW_ATE_signed)
                  DW_AT_byte_size	(0x04)

  0x0000002d:     DW_TAG_LLVM_annotation
                    DW_AT_name	("btf:type_tag")
                    DW_AT_const_value	("tag1")

Such DWARF entries would be used to generate BTF definitions by tools like pahole.

Note: similar fields with similar purposes are already present in DIDerivedType and DICompositeType.

Currently "btf_type_tag" attributes are represented in debug information as 'annotations' fields in DIDerivedType with DW_TAG_pointer_type tag. The annotation on a pointer corresponds to pointee having the attributes in the final BTF.

The discussion in [thread](https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/) came to conclusion, that such annotations should apply to the annotated type itself. Hence the necessity to extend DIBasicType & DISubroutineType types with 'annotations' field to represent cases like below:

  int __attribute__((btf_type_tag("foo"))) bar;

This was previously tracked as differential revision: https://reviews.llvm.org/D143966


Patch is 31.94 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91422.diff

11 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+58-22)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+10-7)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+12-4)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+2)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+5-1)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+4-2)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+8-7)
  • (modified) llvm/lib/IR/LLVMContextImpl.h (+20-10)
  • (added) llvm/test/Bitcode/attr-btf_tag-dibasic.ll (+36)
  • (added) llvm/test/Bitcode/attr-btf_tag-disubroutine.ll (+41)
  • (modified) llvm/test/DebugInfo/attr-btf_type_tag.ll (+99-37)
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 42291d45da2be..1dfaa4aced791 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -828,40 +828,45 @@ class DIBasicType : public DIType {
   static DIBasicType *getImpl(LLVMContext &Context, unsigned Tag,
                               StringRef Name, uint64_t SizeInBits,
                               uint32_t AlignInBits, unsigned Encoding,
-                              DIFlags Flags, StorageType Storage,
-                              bool ShouldCreate = true) {
+                              DIFlags Flags, DINodeArray Annotations,
+                              StorageType Storage, bool ShouldCreate = true) {
     return getImpl(Context, Tag, getCanonicalMDString(Context, Name),
-                   SizeInBits, AlignInBits, Encoding, Flags, Storage,
-                   ShouldCreate);
+                   SizeInBits, AlignInBits, Encoding, Flags, Annotations.get(),
+                   Storage, ShouldCreate);
   }
   static DIBasicType *getImpl(LLVMContext &Context, unsigned Tag,
                               MDString *Name, uint64_t SizeInBits,
                               uint32_t AlignInBits, unsigned Encoding,
-                              DIFlags Flags, StorageType Storage,
-                              bool ShouldCreate = true);
+                              DIFlags Flags, Metadata *Annotations,
+                              StorageType Storage, bool ShouldCreate = true);
 
   TempDIBasicType cloneImpl() const {
     return getTemporary(getContext(), getTag(), getName(), getSizeInBits(),
-                        getAlignInBits(), getEncoding(), getFlags());
+                        getAlignInBits(), getEncoding(), getFlags(),
+                        getAnnotations());
   }
 
 public:
   DEFINE_MDNODE_GET(DIBasicType, (unsigned Tag, StringRef Name),
-                    (Tag, Name, 0, 0, 0, FlagZero))
+                    (Tag, Name, 0, 0, 0, FlagZero, {}))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, StringRef Name, uint64_t SizeInBits),
-                    (Tag, Name, SizeInBits, 0, 0, FlagZero))
+                    (Tag, Name, SizeInBits, 0, 0, FlagZero, {}))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, MDString *Name, uint64_t SizeInBits),
-                    (Tag, Name, SizeInBits, 0, 0, FlagZero))
+                    (Tag, Name, SizeInBits, 0, 0, FlagZero, {}))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, StringRef Name, uint64_t SizeInBits,
-                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags),
-                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags))
+                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+                     DINodeArray Annotations = {}),
+                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags,
+                     Annotations))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, MDString *Name, uint64_t SizeInBits,
-                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags),
-                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags))
+                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+                     Metadata *Annotations = nullptr),
+                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags,
+                     Annotations))
 
   TempDIBasicType clone() const { return cloneImpl(); }
 
@@ -873,6 +878,16 @@ class DIBasicType : public DIType {
   /// neither signed nor unsigned.
   std::optional<Signedness> getSignedness() const;
 
+  Metadata *getRawAnnotations() const { return getOperand(3); }
+
+  DINodeArray getAnnotations() const {
+    return cast_or_null<MDTuple>(getRawAnnotations());
+  }
+
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(3, Annotations.get());
+  }
+
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIBasicTypeKind;
   }
@@ -1112,6 +1127,10 @@ class DIDerivedType : public DIType {
   }
   Metadata *getRawAnnotations() const { return getOperand(5); }
 
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(5, Annotations.get());
+  }
+
   /// Get casted version of extra data.
   /// @{
   DIType *getClassType() const;
@@ -1339,6 +1358,10 @@ class DICompositeType : public DIType {
     return cast_or_null<MDTuple>(getRawAnnotations());
   }
 
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(13, Annotations.get());
+  }
+
   /// Replace operands.
   ///
   /// If this \a isUniqued() and not \a isResolved(), on a uniquing collision
@@ -1385,26 +1408,30 @@ class DISubroutineType : public DIType {
 
   static DISubroutineType *getImpl(LLVMContext &Context, DIFlags Flags,
                                    uint8_t CC, DITypeRefArray TypeArray,
-                                   StorageType Storage,
+                                   DINodeArray Annotations, StorageType Storage,
                                    bool ShouldCreate = true) {
-    return getImpl(Context, Flags, CC, TypeArray.get(), Storage, ShouldCreate);
+    return getImpl(Context, Flags, CC, TypeArray.get(), Annotations.get(),
+                   Storage, ShouldCreate);
   }
   static DISubroutineType *getImpl(LLVMContext &Context, DIFlags Flags,
                                    uint8_t CC, Metadata *TypeArray,
-                                   StorageType Storage,
+                                   Metadata *Annotations, StorageType Storage,
                                    bool ShouldCreate = true);
 
   TempDISubroutineType cloneImpl() const {
-    return getTemporary(getContext(), getFlags(), getCC(), getTypeArray());
+    return getTemporary(getContext(), getFlags(), getCC(), getTypeArray(),
+                        getAnnotations());
   }
 
 public:
   DEFINE_MDNODE_GET(DISubroutineType,
-                    (DIFlags Flags, uint8_t CC, DITypeRefArray TypeArray),
-                    (Flags, CC, TypeArray))
+                    (DIFlags Flags, uint8_t CC, DITypeRefArray TypeArray,
+                     DINodeArray Annotations = nullptr),
+                    (Flags, CC, TypeArray, Annotations))
   DEFINE_MDNODE_GET(DISubroutineType,
-                    (DIFlags Flags, uint8_t CC, Metadata *TypeArray),
-                    (Flags, CC, TypeArray))
+                    (DIFlags Flags, uint8_t CC, Metadata *TypeArray,
+                     Metadata *Annotations = nullptr),
+                    (Flags, CC, TypeArray, Annotations))
 
   TempDISubroutineType clone() const { return cloneImpl(); }
   // Returns a new temporary DISubroutineType with updated CC
@@ -1422,6 +1449,15 @@ class DISubroutineType : public DIType {
 
   Metadata *getRawTypeArray() const { return getOperand(3); }
 
+  Metadata *getRawAnnotations() const { return getOperand(4); }
+  DINodeArray getAnnotations() const {
+    return cast_or_null<MDTuple>(getRawAnnotations());
+  }
+
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(4, Annotations.get());
+  }
+
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DISubroutineTypeKind;
   }
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 34053a5ca9c8e..813454aefd9e1 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -5210,7 +5210,7 @@ bool LLParser::parseDIEnumerator(MDNode *&Result, bool IsDistinct) {
 
 /// parseDIBasicType:
 ///   ::= !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32,
-///                    encoding: DW_ATE_encoding, flags: 0)
+///                    encoding: DW_ATE_encoding, flags: 0, annotations: !1)
 bool LLParser::parseDIBasicType(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   OPTIONAL(tag, DwarfTagField, (dwarf::DW_TAG_base_type));                     \
@@ -5218,12 +5218,14 @@ bool LLParser::parseDIBasicType(MDNode *&Result, bool IsDistinct) {
   OPTIONAL(size, MDUnsignedField, (0, UINT64_MAX));                            \
   OPTIONAL(align, MDUnsignedField, (0, UINT32_MAX));                           \
   OPTIONAL(encoding, DwarfAttEncodingField, );                                 \
-  OPTIONAL(flags, DIFlagField, );
+  OPTIONAL(flags, DIFlagField, );                                              \
+  OPTIONAL(annotations, MDField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
-  Result = GET_OR_DISTINCT(DIBasicType, (Context, tag.Val, name.Val, size.Val,
-                                         align.Val, encoding.Val, flags.Val));
+  Result = GET_OR_DISTINCT(DIBasicType,
+                           (Context, tag.Val, name.Val, size.Val, align.Val,
+                            encoding.Val, flags.Val, annotations.Val));
   return false;
 }
 
@@ -5360,12 +5362,13 @@ bool LLParser::parseDISubroutineType(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(cc, DwarfCCField, );                                                \
-  REQUIRED(types, MDField, );
+  REQUIRED(types, MDField, );                                                  \
+  OPTIONAL(annotations, MDField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
-  Result = GET_OR_DISTINCT(DISubroutineType,
-                           (Context, flags.Val, cc.Val, types.Val));
+  Result = GET_OR_DISTINCT(DISubroutineType, (Context, flags.Val, cc.Val,
+                                              types.Val, annotations.Val));
   return false;
 }
 
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 9102f3a60cffc..bc06c55f1662c 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -1527,7 +1527,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     break;
   }
   case bitc::METADATA_BASIC_TYPE: {
-    if (Record.size() < 6 || Record.size() > 7)
+    if (Record.size() < 6 || Record.size() > 8)
       return error("Invalid record");
 
     IsDistinct = Record[0];
@@ -1535,10 +1535,14 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
                                 ? static_cast<DINode::DIFlags>(Record[6])
                                 : DINode::FlagZero;
 
+    Metadata *Annotations = nullptr;
+    if (Record.size() > 7 && Record[7])
+      Annotations = getMDOrNull(Record[7]);
+
     MetadataList.assignValue(
         GET_OR_DISTINCT(DIBasicType,
                         (Context, Record[1], getMDString(Record[2]), Record[3],
-                         Record[4], Record[5], Flags)),
+                         Record[4], Record[5], Flags, Annotations)),
         NextMetadataNo);
     NextMetadataNo++;
     break;
@@ -1703,7 +1707,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     break;
   }
   case bitc::METADATA_SUBROUTINE_TYPE: {
-    if (Record.size() < 3 || Record.size() > 4)
+    if (Record.size() < 3 || Record.size() > 5)
       return error("Invalid record");
     bool IsOldTypeRefArray = Record[0] < 2;
     unsigned CC = (Record.size() > 3) ? Record[3] : 0;
@@ -1713,9 +1717,13 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     Metadata *Types = getMDOrNull(Record[2]);
     if (LLVM_UNLIKELY(IsOldTypeRefArray))
       Types = MetadataList.upgradeTypeRefArray(Types);
+    Metadata *Annotations = nullptr;
+    if (Record.size() > 4 && Record[4])
+      Annotations = getMDOrNull(Record[4]);
 
     MetadataList.assignValue(
-        GET_OR_DISTINCT(DISubroutineType, (Context, Flags, CC, Types)),
+        GET_OR_DISTINCT(DISubroutineType,
+                        (Context, Flags, CC, Types, Annotations)),
         NextMetadataNo);
     NextMetadataNo++;
     break;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 6d01e3b4d8218..d7b5c4f7f3a1f 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1798,6 +1798,7 @@ void ModuleBitcodeWriter::writeDIBasicType(const DIBasicType *N,
   Record.push_back(N->getAlignInBits());
   Record.push_back(N->getEncoding());
   Record.push_back(N->getFlags());
+  Record.push_back(VE.getMetadataOrNullID(N->getRawAnnotations()));
 
   Stream.EmitRecord(bitc::METADATA_BASIC_TYPE, Record, Abbrev);
   Record.clear();
@@ -1893,6 +1894,7 @@ void ModuleBitcodeWriter::writeDISubroutineType(
   Record.push_back(N->getFlags());
   Record.push_back(VE.getMetadataOrNullID(N->getTypeArray().get()));
   Record.push_back(N->getCC());
+  Record.push_back(VE.getMetadataOrNullID(N->getRawAnnotations()));
 
   Stream.EmitRecord(bitc::METADATA_SUBROUTINE_TYPE, Record, Abbrev);
   Record.clear();
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index 56c288ee95b43..203f496536002 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -696,7 +696,9 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DIBasicType *BTy) {
   if (!Name.empty())
     addString(Buffer, dwarf::DW_AT_name, Name);
 
-  // An unspecified type only has a name attribute.
+  addAnnotation(Buffer, BTy->getAnnotations());
+
+  // An unspecified type only has a name attribute & annotations.
   if (BTy->getTag() == dwarf::DW_TAG_unspecified_type)
     return;
 
@@ -865,6 +867,8 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DISubroutineType *CTy) {
 
   if (CTy->isRValueReference())
     addFlag(Buffer, dwarf::DW_AT_rvalue_reference);
+
+  addAnnotation(Buffer, CTy->getAnnotations());
 }
 
 void DwarfUnit::addAnnotation(DIE &Buffer, DINodeArray Annotations) {
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 941f6a7a7d823..b37c01a10d025 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2096,9 +2096,9 @@ static void writeDIEnumerator(raw_ostream &Out, const DIEnumerator *N,
 }
 
 static void writeDIBasicType(raw_ostream &Out, const DIBasicType *N,
-                             AsmWriterContext &) {
+                             AsmWriterContext &WriterCtx) {
   Out << "!DIBasicType(";
-  MDFieldPrinter Printer(Out);
+  MDFieldPrinter Printer(Out, WriterCtx);
   if (N->getTag() != dwarf::DW_TAG_base_type)
     Printer.printTag(N);
   Printer.printString("name", N->getName());
@@ -2107,6 +2107,7 @@ static void writeDIBasicType(raw_ostream &Out, const DIBasicType *N,
   Printer.printDwarfEnum("encoding", N->getEncoding(),
                          dwarf::AttributeEncodingString);
   Printer.printDIFlags("flags", N->getFlags());
+  Printer.printMetadata("annotations", N->getRawAnnotations());
   Out << ")";
 }
 
@@ -2202,6 +2203,7 @@ static void writeDISubroutineType(raw_ostream &Out, const DISubroutineType *N,
   Printer.printDwarfEnum("cc", N->getCC(), dwarf::ConventionString);
   Printer.printMetadata("types", N->getRawTypeArray(),
                         /* ShouldSkipNull */ false);
+  Printer.printMetadata("annotations", N->getRawAnnotations());
   Out << ")";
 }
 
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 570515505607f..264d8e467fde0 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -663,12 +663,12 @@ DIEnumerator *DIEnumerator::getImpl(LLVMContext &Context, const APInt &Value,
 DIBasicType *DIBasicType::getImpl(LLVMContext &Context, unsigned Tag,
                                   MDString *Name, uint64_t SizeInBits,
                                   uint32_t AlignInBits, unsigned Encoding,
-                                  DIFlags Flags, StorageType Storage,
-                                  bool ShouldCreate) {
+                                  DIFlags Flags, Metadata *Annotations,
+                                  StorageType Storage, bool ShouldCreate) {
   assert(isCanonical(Name) && "Expected canonical MDString");
-  DEFINE_GETIMPL_LOOKUP(DIBasicType,
-                        (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags));
-  Metadata *Ops[] = {nullptr, nullptr, Name};
+  DEFINE_GETIMPL_LOOKUP(DIBasicType, (Tag, Name, SizeInBits, AlignInBits,
+                                      Encoding, Flags, Annotations));
+  Metadata *Ops[] = {nullptr, nullptr, Name, Annotations};
   DEFINE_GETIMPL_STORE(DIBasicType,
                        (Tag, SizeInBits, AlignInBits, Encoding, Flags), Ops);
 }
@@ -872,10 +872,11 @@ DISubroutineType::DISubroutineType(LLVMContext &C, StorageType Storage,
 
 DISubroutineType *DISubroutineType::getImpl(LLVMContext &Context, DIFlags Flags,
                                             uint8_t CC, Metadata *TypeArray,
+                                            Metadata *Annotations,
                                             StorageType Storage,
                                             bool ShouldCreate) {
-  DEFINE_GETIMPL_LOOKUP(DISubroutineType, (Flags, CC, TypeArray));
-  Metadata *Ops[] = {nullptr, nullptr, nullptr, TypeArray};
+  DEFINE_GETIMPL_LOOKUP(DISubroutineType, (Flags, CC, TypeArray, Annotations));
+  Metadata *Ops[] = {nullptr, nullptr, nullptr, TypeArray, Annotations};
   DEFINE_GETIMPL_STORE(DISubroutineType, (Flags, CC), Ops);
 }
 
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index 399fe0dad26c7..86bfcce599e77 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -465,25 +465,29 @@ template <> struct MDNodeKeyImpl<DIBasicType> {
   uint32_t AlignInBits;
   unsigned Encoding;
   unsigned Flags;
+  Metadata *Annotations;
 
   MDNodeKeyImpl(unsigned Tag, MDString *Name, uint64_t SizeInBits,
-                uint32_t AlignInBits, unsigned Encoding, unsigned Flags)
+                uint32_t AlignInBits, unsigned Encoding, unsigned Flags,
+                Metadata *Annotations)
       : Tag(Tag), Name(Name), SizeInBits(SizeInBits), AlignInBits(AlignInBits),
-        Encoding(Encoding), Flags(Flags) {}
+        Encoding(Encoding), Flags(Flags), Annotations(Annotations) {}
   MDNodeKeyImpl(const DIBasicType *N)
       : Tag(N->getTag()), Name(N->getRawName()), SizeInBits(N->getSizeInBits()),
         AlignInBits(N->getAlignInBits()), Encoding(N->getEncoding()),
-        Flags(N->getFlags()) {}
+        Flags(N->getFlags()), Annotations(N->getRawAnnotations()) {}
 
   bool isKeyOf(const DIBasicType *RHS) const {
     return Tag == RHS->getTag() && Name == RHS->getRawName() &&
            SizeInBits == RHS->getSizeInBits() &&
            AlignInBits == RHS->getAlignInBits() &&
-           Encoding == RHS->getEncoding() && Flags == RHS->getFlags();
+           Encoding == RHS->getEncoding() && Flags == RHS->getFlags() &&
+           Annotations == RHS->getRawAnnotations();
   }
 
   unsigned getHashValue() const {
-    return hash_combine(Tag, Name, SizeInBits, AlignInBits, Encoding);
+    return hash_combine(Tag, Name, SizeInBits, AlignInBits, Encoding,
+                        Annotations);
   }
 };
 
@@ -712,18 +716,24 @@ template <> struct MDNodeKeyImpl<DISubroutineType> {
   unsigned Flags;
   uint8_t CC;
   Metadata *TypeArray;
+  Metadata *Annotations;
 
-  MDNodeKeyImpl(unsigned Flags, uint8_t CC, Metadata *TypeArray)
-      : Flags(Flags), CC(CC), TypeArray(TypeArray) {}
+  MDNodeKeyImpl(unsigned Flags, uint8_t CC, Metadata *TypeArray,
+                Metadata *Annotations)
+      : Flags(Flags), CC(CC), TypeArray(TypeArray), Annotations(Annotations) {}
   MDNodeKeyImpl(const DISubroutineType *N)
-      : Flags(N->getFlags()), CC(N->getCC()), TypeArray(N->getRawTypeArray()) {}
+      : Flags(N->getFlags()), CC(N->getCC()), TypeArray(N->getRawTypeArray()),
+        Annotations(N->getRawAnnotations()) {}
 
   bool isKeyOf(const DISubroutineType *RHS) const {
     return Flags == RHS->getFlags() && CC == RHS->getCC() &&
-           TypeArray == RHS->getRawTypeArray();
+           TypeArray == RHS->getRawTypeArray() &&
+           Annotations == RHS->getRawAnnotations();
   }
 
-  unsigned getHashValue() const { return hash_combine(Flags, CC, TypeArray); }
+  unsigned getHashValue() const {
+    return hash_combine(Flags, CC, TypeArray, Anno...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented May 8, 2024

@llvm/pr-subscribers-debuginfo

Author: None (eddyz87)

Changes

Extend DIBasicType and DISubroutineType with additional field annotations, e.g. as below:

  !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed, annotations: !6)
  !6 = !{!7}
  !7 = !{!"btf:type_tag", !"tag1"}

The field would be used by BPF backend to generate DWARF attributes corresponding to btf_type_tag type attributes, e.g.:

  0x00000029:   DW_TAG_base_type
                  DW_AT_name	("int")
                  DW_AT_encoding	(DW_ATE_signed)
                  DW_AT_byte_size	(0x04)

  0x0000002d:     DW_TAG_LLVM_annotation
                    DW_AT_name	("btf:type_tag")
                    DW_AT_const_value	("tag1")

Such DWARF entries would be used to generate BTF definitions by tools like pahole.

Note: similar fields with similar purposes are already present in DIDerivedType and DICompositeType.

Currently "btf_type_tag" attributes are represented in debug information as 'annotations' fields in DIDerivedType with DW_TAG_pointer_type tag. The annotation on a pointer corresponds to pointee having the attributes in the final BTF.

The discussion in [thread](https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/) came to conclusion, that such annotations should apply to the annotated type itself. Hence the necessity to extend DIBasicType & DISubroutineType types with 'annotations' field to represent cases like below:

  int __attribute__((btf_type_tag("foo"))) bar;

This was previously tracked as differential revision: https://reviews.llvm.org/D143966


Patch is 31.94 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91422.diff

11 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+58-22)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+10-7)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+12-4)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+2)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+5-1)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+4-2)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+8-7)
  • (modified) llvm/lib/IR/LLVMContextImpl.h (+20-10)
  • (added) llvm/test/Bitcode/attr-btf_tag-dibasic.ll (+36)
  • (added) llvm/test/Bitcode/attr-btf_tag-disubroutine.ll (+41)
  • (modified) llvm/test/DebugInfo/attr-btf_type_tag.ll (+99-37)
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 42291d45da2be..1dfaa4aced791 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -828,40 +828,45 @@ class DIBasicType : public DIType {
   static DIBasicType *getImpl(LLVMContext &Context, unsigned Tag,
                               StringRef Name, uint64_t SizeInBits,
                               uint32_t AlignInBits, unsigned Encoding,
-                              DIFlags Flags, StorageType Storage,
-                              bool ShouldCreate = true) {
+                              DIFlags Flags, DINodeArray Annotations,
+                              StorageType Storage, bool ShouldCreate = true) {
     return getImpl(Context, Tag, getCanonicalMDString(Context, Name),
-                   SizeInBits, AlignInBits, Encoding, Flags, Storage,
-                   ShouldCreate);
+                   SizeInBits, AlignInBits, Encoding, Flags, Annotations.get(),
+                   Storage, ShouldCreate);
   }
   static DIBasicType *getImpl(LLVMContext &Context, unsigned Tag,
                               MDString *Name, uint64_t SizeInBits,
                               uint32_t AlignInBits, unsigned Encoding,
-                              DIFlags Flags, StorageType Storage,
-                              bool ShouldCreate = true);
+                              DIFlags Flags, Metadata *Annotations,
+                              StorageType Storage, bool ShouldCreate = true);
 
   TempDIBasicType cloneImpl() const {
     return getTemporary(getContext(), getTag(), getName(), getSizeInBits(),
-                        getAlignInBits(), getEncoding(), getFlags());
+                        getAlignInBits(), getEncoding(), getFlags(),
+                        getAnnotations());
   }
 
 public:
   DEFINE_MDNODE_GET(DIBasicType, (unsigned Tag, StringRef Name),
-                    (Tag, Name, 0, 0, 0, FlagZero))
+                    (Tag, Name, 0, 0, 0, FlagZero, {}))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, StringRef Name, uint64_t SizeInBits),
-                    (Tag, Name, SizeInBits, 0, 0, FlagZero))
+                    (Tag, Name, SizeInBits, 0, 0, FlagZero, {}))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, MDString *Name, uint64_t SizeInBits),
-                    (Tag, Name, SizeInBits, 0, 0, FlagZero))
+                    (Tag, Name, SizeInBits, 0, 0, FlagZero, {}))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, StringRef Name, uint64_t SizeInBits,
-                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags),
-                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags))
+                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+                     DINodeArray Annotations = {}),
+                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags,
+                     Annotations))
   DEFINE_MDNODE_GET(DIBasicType,
                     (unsigned Tag, MDString *Name, uint64_t SizeInBits,
-                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags),
-                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags))
+                     uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+                     Metadata *Annotations = nullptr),
+                    (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags,
+                     Annotations))
 
   TempDIBasicType clone() const { return cloneImpl(); }
 
@@ -873,6 +878,16 @@ class DIBasicType : public DIType {
   /// neither signed nor unsigned.
   std::optional<Signedness> getSignedness() const;
 
+  Metadata *getRawAnnotations() const { return getOperand(3); }
+
+  DINodeArray getAnnotations() const {
+    return cast_or_null<MDTuple>(getRawAnnotations());
+  }
+
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(3, Annotations.get());
+  }
+
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIBasicTypeKind;
   }
@@ -1112,6 +1127,10 @@ class DIDerivedType : public DIType {
   }
   Metadata *getRawAnnotations() const { return getOperand(5); }
 
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(5, Annotations.get());
+  }
+
   /// Get casted version of extra data.
   /// @{
   DIType *getClassType() const;
@@ -1339,6 +1358,10 @@ class DICompositeType : public DIType {
     return cast_or_null<MDTuple>(getRawAnnotations());
   }
 
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(13, Annotations.get());
+  }
+
   /// Replace operands.
   ///
   /// If this \a isUniqued() and not \a isResolved(), on a uniquing collision
@@ -1385,26 +1408,30 @@ class DISubroutineType : public DIType {
 
   static DISubroutineType *getImpl(LLVMContext &Context, DIFlags Flags,
                                    uint8_t CC, DITypeRefArray TypeArray,
-                                   StorageType Storage,
+                                   DINodeArray Annotations, StorageType Storage,
                                    bool ShouldCreate = true) {
-    return getImpl(Context, Flags, CC, TypeArray.get(), Storage, ShouldCreate);
+    return getImpl(Context, Flags, CC, TypeArray.get(), Annotations.get(),
+                   Storage, ShouldCreate);
   }
   static DISubroutineType *getImpl(LLVMContext &Context, DIFlags Flags,
                                    uint8_t CC, Metadata *TypeArray,
-                                   StorageType Storage,
+                                   Metadata *Annotations, StorageType Storage,
                                    bool ShouldCreate = true);
 
   TempDISubroutineType cloneImpl() const {
-    return getTemporary(getContext(), getFlags(), getCC(), getTypeArray());
+    return getTemporary(getContext(), getFlags(), getCC(), getTypeArray(),
+                        getAnnotations());
   }
 
 public:
   DEFINE_MDNODE_GET(DISubroutineType,
-                    (DIFlags Flags, uint8_t CC, DITypeRefArray TypeArray),
-                    (Flags, CC, TypeArray))
+                    (DIFlags Flags, uint8_t CC, DITypeRefArray TypeArray,
+                     DINodeArray Annotations = nullptr),
+                    (Flags, CC, TypeArray, Annotations))
   DEFINE_MDNODE_GET(DISubroutineType,
-                    (DIFlags Flags, uint8_t CC, Metadata *TypeArray),
-                    (Flags, CC, TypeArray))
+                    (DIFlags Flags, uint8_t CC, Metadata *TypeArray,
+                     Metadata *Annotations = nullptr),
+                    (Flags, CC, TypeArray, Annotations))
 
   TempDISubroutineType clone() const { return cloneImpl(); }
   // Returns a new temporary DISubroutineType with updated CC
@@ -1422,6 +1449,15 @@ class DISubroutineType : public DIType {
 
   Metadata *getRawTypeArray() const { return getOperand(3); }
 
+  Metadata *getRawAnnotations() const { return getOperand(4); }
+  DINodeArray getAnnotations() const {
+    return cast_or_null<MDTuple>(getRawAnnotations());
+  }
+
+  void replaceAnnotations(DINodeArray Annotations) {
+    replaceOperandWith(4, Annotations.get());
+  }
+
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DISubroutineTypeKind;
   }
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 34053a5ca9c8e..813454aefd9e1 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -5210,7 +5210,7 @@ bool LLParser::parseDIEnumerator(MDNode *&Result, bool IsDistinct) {
 
 /// parseDIBasicType:
 ///   ::= !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32,
-///                    encoding: DW_ATE_encoding, flags: 0)
+///                    encoding: DW_ATE_encoding, flags: 0, annotations: !1)
 bool LLParser::parseDIBasicType(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   OPTIONAL(tag, DwarfTagField, (dwarf::DW_TAG_base_type));                     \
@@ -5218,12 +5218,14 @@ bool LLParser::parseDIBasicType(MDNode *&Result, bool IsDistinct) {
   OPTIONAL(size, MDUnsignedField, (0, UINT64_MAX));                            \
   OPTIONAL(align, MDUnsignedField, (0, UINT32_MAX));                           \
   OPTIONAL(encoding, DwarfAttEncodingField, );                                 \
-  OPTIONAL(flags, DIFlagField, );
+  OPTIONAL(flags, DIFlagField, );                                              \
+  OPTIONAL(annotations, MDField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
-  Result = GET_OR_DISTINCT(DIBasicType, (Context, tag.Val, name.Val, size.Val,
-                                         align.Val, encoding.Val, flags.Val));
+  Result = GET_OR_DISTINCT(DIBasicType,
+                           (Context, tag.Val, name.Val, size.Val, align.Val,
+                            encoding.Val, flags.Val, annotations.Val));
   return false;
 }
 
@@ -5360,12 +5362,13 @@ bool LLParser::parseDISubroutineType(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(cc, DwarfCCField, );                                                \
-  REQUIRED(types, MDField, );
+  REQUIRED(types, MDField, );                                                  \
+  OPTIONAL(annotations, MDField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
-  Result = GET_OR_DISTINCT(DISubroutineType,
-                           (Context, flags.Val, cc.Val, types.Val));
+  Result = GET_OR_DISTINCT(DISubroutineType, (Context, flags.Val, cc.Val,
+                                              types.Val, annotations.Val));
   return false;
 }
 
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 9102f3a60cffc..bc06c55f1662c 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -1527,7 +1527,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     break;
   }
   case bitc::METADATA_BASIC_TYPE: {
-    if (Record.size() < 6 || Record.size() > 7)
+    if (Record.size() < 6 || Record.size() > 8)
       return error("Invalid record");
 
     IsDistinct = Record[0];
@@ -1535,10 +1535,14 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
                                 ? static_cast<DINode::DIFlags>(Record[6])
                                 : DINode::FlagZero;
 
+    Metadata *Annotations = nullptr;
+    if (Record.size() > 7 && Record[7])
+      Annotations = getMDOrNull(Record[7]);
+
     MetadataList.assignValue(
         GET_OR_DISTINCT(DIBasicType,
                         (Context, Record[1], getMDString(Record[2]), Record[3],
-                         Record[4], Record[5], Flags)),
+                         Record[4], Record[5], Flags, Annotations)),
         NextMetadataNo);
     NextMetadataNo++;
     break;
@@ -1703,7 +1707,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     break;
   }
   case bitc::METADATA_SUBROUTINE_TYPE: {
-    if (Record.size() < 3 || Record.size() > 4)
+    if (Record.size() < 3 || Record.size() > 5)
       return error("Invalid record");
     bool IsOldTypeRefArray = Record[0] < 2;
     unsigned CC = (Record.size() > 3) ? Record[3] : 0;
@@ -1713,9 +1717,13 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     Metadata *Types = getMDOrNull(Record[2]);
     if (LLVM_UNLIKELY(IsOldTypeRefArray))
       Types = MetadataList.upgradeTypeRefArray(Types);
+    Metadata *Annotations = nullptr;
+    if (Record.size() > 4 && Record[4])
+      Annotations = getMDOrNull(Record[4]);
 
     MetadataList.assignValue(
-        GET_OR_DISTINCT(DISubroutineType, (Context, Flags, CC, Types)),
+        GET_OR_DISTINCT(DISubroutineType,
+                        (Context, Flags, CC, Types, Annotations)),
         NextMetadataNo);
     NextMetadataNo++;
     break;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 6d01e3b4d8218..d7b5c4f7f3a1f 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1798,6 +1798,7 @@ void ModuleBitcodeWriter::writeDIBasicType(const DIBasicType *N,
   Record.push_back(N->getAlignInBits());
   Record.push_back(N->getEncoding());
   Record.push_back(N->getFlags());
+  Record.push_back(VE.getMetadataOrNullID(N->getRawAnnotations()));
 
   Stream.EmitRecord(bitc::METADATA_BASIC_TYPE, Record, Abbrev);
   Record.clear();
@@ -1893,6 +1894,7 @@ void ModuleBitcodeWriter::writeDISubroutineType(
   Record.push_back(N->getFlags());
   Record.push_back(VE.getMetadataOrNullID(N->getTypeArray().get()));
   Record.push_back(N->getCC());
+  Record.push_back(VE.getMetadataOrNullID(N->getRawAnnotations()));
 
   Stream.EmitRecord(bitc::METADATA_SUBROUTINE_TYPE, Record, Abbrev);
   Record.clear();
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index 56c288ee95b43..203f496536002 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -696,7 +696,9 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DIBasicType *BTy) {
   if (!Name.empty())
     addString(Buffer, dwarf::DW_AT_name, Name);
 
-  // An unspecified type only has a name attribute.
+  addAnnotation(Buffer, BTy->getAnnotations());
+
+  // An unspecified type only has a name attribute & annotations.
   if (BTy->getTag() == dwarf::DW_TAG_unspecified_type)
     return;
 
@@ -865,6 +867,8 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DISubroutineType *CTy) {
 
   if (CTy->isRValueReference())
     addFlag(Buffer, dwarf::DW_AT_rvalue_reference);
+
+  addAnnotation(Buffer, CTy->getAnnotations());
 }
 
 void DwarfUnit::addAnnotation(DIE &Buffer, DINodeArray Annotations) {
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 941f6a7a7d823..b37c01a10d025 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2096,9 +2096,9 @@ static void writeDIEnumerator(raw_ostream &Out, const DIEnumerator *N,
 }
 
 static void writeDIBasicType(raw_ostream &Out, const DIBasicType *N,
-                             AsmWriterContext &) {
+                             AsmWriterContext &WriterCtx) {
   Out << "!DIBasicType(";
-  MDFieldPrinter Printer(Out);
+  MDFieldPrinter Printer(Out, WriterCtx);
   if (N->getTag() != dwarf::DW_TAG_base_type)
     Printer.printTag(N);
   Printer.printString("name", N->getName());
@@ -2107,6 +2107,7 @@ static void writeDIBasicType(raw_ostream &Out, const DIBasicType *N,
   Printer.printDwarfEnum("encoding", N->getEncoding(),
                          dwarf::AttributeEncodingString);
   Printer.printDIFlags("flags", N->getFlags());
+  Printer.printMetadata("annotations", N->getRawAnnotations());
   Out << ")";
 }
 
@@ -2202,6 +2203,7 @@ static void writeDISubroutineType(raw_ostream &Out, const DISubroutineType *N,
   Printer.printDwarfEnum("cc", N->getCC(), dwarf::ConventionString);
   Printer.printMetadata("types", N->getRawTypeArray(),
                         /* ShouldSkipNull */ false);
+  Printer.printMetadata("annotations", N->getRawAnnotations());
   Out << ")";
 }
 
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 570515505607f..264d8e467fde0 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -663,12 +663,12 @@ DIEnumerator *DIEnumerator::getImpl(LLVMContext &Context, const APInt &Value,
 DIBasicType *DIBasicType::getImpl(LLVMContext &Context, unsigned Tag,
                                   MDString *Name, uint64_t SizeInBits,
                                   uint32_t AlignInBits, unsigned Encoding,
-                                  DIFlags Flags, StorageType Storage,
-                                  bool ShouldCreate) {
+                                  DIFlags Flags, Metadata *Annotations,
+                                  StorageType Storage, bool ShouldCreate) {
   assert(isCanonical(Name) && "Expected canonical MDString");
-  DEFINE_GETIMPL_LOOKUP(DIBasicType,
-                        (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags));
-  Metadata *Ops[] = {nullptr, nullptr, Name};
+  DEFINE_GETIMPL_LOOKUP(DIBasicType, (Tag, Name, SizeInBits, AlignInBits,
+                                      Encoding, Flags, Annotations));
+  Metadata *Ops[] = {nullptr, nullptr, Name, Annotations};
   DEFINE_GETIMPL_STORE(DIBasicType,
                        (Tag, SizeInBits, AlignInBits, Encoding, Flags), Ops);
 }
@@ -872,10 +872,11 @@ DISubroutineType::DISubroutineType(LLVMContext &C, StorageType Storage,
 
 DISubroutineType *DISubroutineType::getImpl(LLVMContext &Context, DIFlags Flags,
                                             uint8_t CC, Metadata *TypeArray,
+                                            Metadata *Annotations,
                                             StorageType Storage,
                                             bool ShouldCreate) {
-  DEFINE_GETIMPL_LOOKUP(DISubroutineType, (Flags, CC, TypeArray));
-  Metadata *Ops[] = {nullptr, nullptr, nullptr, TypeArray};
+  DEFINE_GETIMPL_LOOKUP(DISubroutineType, (Flags, CC, TypeArray, Annotations));
+  Metadata *Ops[] = {nullptr, nullptr, nullptr, TypeArray, Annotations};
   DEFINE_GETIMPL_STORE(DISubroutineType, (Flags, CC), Ops);
 }
 
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index 399fe0dad26c7..86bfcce599e77 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -465,25 +465,29 @@ template <> struct MDNodeKeyImpl<DIBasicType> {
   uint32_t AlignInBits;
   unsigned Encoding;
   unsigned Flags;
+  Metadata *Annotations;
 
   MDNodeKeyImpl(unsigned Tag, MDString *Name, uint64_t SizeInBits,
-                uint32_t AlignInBits, unsigned Encoding, unsigned Flags)
+                uint32_t AlignInBits, unsigned Encoding, unsigned Flags,
+                Metadata *Annotations)
       : Tag(Tag), Name(Name), SizeInBits(SizeInBits), AlignInBits(AlignInBits),
-        Encoding(Encoding), Flags(Flags) {}
+        Encoding(Encoding), Flags(Flags), Annotations(Annotations) {}
   MDNodeKeyImpl(const DIBasicType *N)
       : Tag(N->getTag()), Name(N->getRawName()), SizeInBits(N->getSizeInBits()),
         AlignInBits(N->getAlignInBits()), Encoding(N->getEncoding()),
-        Flags(N->getFlags()) {}
+        Flags(N->getFlags()), Annotations(N->getRawAnnotations()) {}
 
   bool isKeyOf(const DIBasicType *RHS) const {
     return Tag == RHS->getTag() && Name == RHS->getRawName() &&
            SizeInBits == RHS->getSizeInBits() &&
            AlignInBits == RHS->getAlignInBits() &&
-           Encoding == RHS->getEncoding() && Flags == RHS->getFlags();
+           Encoding == RHS->getEncoding() && Flags == RHS->getFlags() &&
+           Annotations == RHS->getRawAnnotations();
   }
 
   unsigned getHashValue() const {
-    return hash_combine(Tag, Name, SizeInBits, AlignInBits, Encoding);
+    return hash_combine(Tag, Name, SizeInBits, AlignInBits, Encoding,
+                        Annotations);
   }
 };
 
@@ -712,18 +716,24 @@ template <> struct MDNodeKeyImpl<DISubroutineType> {
   unsigned Flags;
   uint8_t CC;
   Metadata *TypeArray;
+  Metadata *Annotations;
 
-  MDNodeKeyImpl(unsigned Flags, uint8_t CC, Metadata *TypeArray)
-      : Flags(Flags), CC(CC), TypeArray(TypeArray) {}
+  MDNodeKeyImpl(unsigned Flags, uint8_t CC, Metadata *TypeArray,
+                Metadata *Annotations)
+      : Flags(Flags), CC(CC), TypeArray(TypeArray), Annotations(Annotations) {}
   MDNodeKeyImpl(const DISubroutineType *N)
-      : Flags(N->getFlags()), CC(N->getCC()), TypeArray(N->getRawTypeArray()) {}
+      : Flags(N->getFlags()), CC(N->getCC()), TypeArray(N->getRawTypeArray()),
+        Annotations(N->getRawAnnotations()) {}
 
   bool isKeyOf(const DISubroutineType *RHS) const {
     return Flags == RHS->getFlags() && CC == RHS->getCC() &&
-           TypeArray == RHS->getRawTypeArray();
+           TypeArray == RHS->getRawTypeArray() &&
+           Annotations == RHS->getRawAnnotations();
   }
 
-  unsigned getHashValue() const { return hash_combine(Flags, CC, TypeArray); }
+  unsigned getHashValue() const {
+    return hash_combine(Flags, CC, TypeArray, Anno...
[truncated]

@eddyz87
Copy link
Contributor Author

eddyz87 commented May 16, 2024

Hi, @dwblaikie, kindly a ping for this thread.

Hi @dafaust, @jemarch, please take a look at the tests for this pull request stack, I'd like to know if we are still on the same page with GCC.

@yonghong-song
Copy link
Contributor

This pull request is the same as the patch in https://reviews.llvm.org/D143966 which has been acked by @dwblaikie and it looks good to me as well. So approve this pull request. @dwblaikie If you got a chance, could you take a look again?

@eddyz87
Copy link
Contributor Author

eddyz87 commented May 23, 2024

@dwblaikie, could you please take a look?

@eddyz87 eddyz87 force-pushed the btf-type-tag-v2-more-di-annotations branch from 4f3fbf2 to 08705ac Compare June 5, 2024 06:48
…neType

Extend DIBasicType and DISubroutineType with additional field
'annotations', e.g. as below:

  !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed, annotations: !6)
  !6 = !{!7}
  !7 = !{!"btf:type_tag", !"tag1"}

The field would be used by BPF backend to generate DWARF attributes
corresponding to "btf_type_tag" type attributes, e.g.:

  0x00000029:   DW_TAG_base_type
                  DW_AT_name	("int")
                  DW_AT_encoding	(DW_ATE_signed)
                  DW_AT_byte_size	(0x04)

  0x0000002d:     DW_TAG_LLVM_annotation
                    DW_AT_name	("btf:type_tag")
                    DW_AT_const_value	("tag1")

Such DWARF entries would be used to generate BTF definitions by tools
like pahole [1].

Note: similar fields with similar purposes are already present in
DIDerivedType and DICompositeType.

Currently "btf_type_tag" attributes are represented in debug
information as 'annotations' fields in DIDerivedType with
DW_TAG_pointer_type tag. The annotation on a pointer corresponds to
pointee having the attributes in the final BTF.

The discussion at [2] came to conclusion, that such annotations should
apply to the annotated type itself. Hence the necessity to extend
DIBasicType & DISubroutineType types with 'annotations' field to
represent cases like below:

  int __attribute__((btf_type_tag("foo"))) bar;

[1] https://github.com/acmel/dwarves
[2] https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/

This was previously tracked as differential revision:
https://reviews.llvm.org/D143966
@eddyz87 eddyz87 force-pushed the btf-type-tag-v2-more-di-annotations branch from 08705ac to 475699f Compare June 12, 2024 17:43
@eddyz87 eddyz87 merged commit 3ca1744 into llvm:main Jun 18, 2024
7 checks passed
@nikic
Copy link
Contributor

nikic commented Jun 18, 2024

@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 18, 2024

It looks like this adds some non-trivial overhead to LTO + debuginfo builds: http://llvm-compile-time-tracker.com/compare.php?from=d95b82c49aef0223bcc03ff5a9163e70037c82be&to=3ca17443ef4af21bdb1f3b4fbcfff672cbc6176c&stat=instructions:u

Oh... I'll try to reproduce locally, but that is bad. The commit seems as vanilla as it gets, if the overhead is caused by an extra action in constructor I'm not sure about remedial options.

@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 19, 2024

I tried building using LTO config -gdwarf-4 -fomit-frame-pointer -flto -DNDEBUG (dwarf4 is for callgrind to work), host compiler is clang-18, linker ld.lld. Turns out, 32Gb machine is not sufficient to link the clang-19 executable, process runs out of memory.
Are there any tricks to make this work?

@nikic
Copy link
Contributor

nikic commented Jun 19, 2024

@eddyz87 Sorry, the "LTO + debuginfo" here referred to the configuration of llvm-test-suite (ReleaseLTO-g cmake cache file), not for the clang compiler. For those particular results the clang compiler is a plain build (no LTO) with gcc host.

@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 19, 2024

@nikic , you mean RelWithDebInfo or Release w/o additional flags, right?
Could you please point me to the script that builds LLVM on the test bot, for some reason I can't figure out which part of the llvm-compile-time-tracker does it...

@azhan92
Copy link
Contributor

azhan92 commented Jun 19, 2024

Hello! I'm seeing the following test failure on AIX that seems to have been caused by this commit:

******************** TEST 'LLVM :: DebugInfo/attr-btf_type_tag.ll' FAILED ********************
Exit Code: 134
Command Output (stderr):
--
RUN: at line 2: /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/llc -filetype=obj -o /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/test/DebugInfo/Output/attr-btf_type_tag.ll.tmp /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/DebugInfo/attr-btf_type_tag.ll
+ /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/llc -filetype=obj -o /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/test/DebugInfo/Output/attr-btf_type_tag.ll.tmp /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/DebugInfo/attr-btf_type_tag.ll
Assertion failed: Section && "Cannot switch to a null section!", file  /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/lib/MC/MCStreamer.cpp, line 1246, virtual void llvm::MCStreamer::switchSection(MCSection *, const MCExpr *)()
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/llc -filetype=obj -o /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/test/DebugInfo/Output/attr-btf_type_tag.ll.tmp /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/DebugInfo/attr-btf_type_tag.ll
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/test/DebugInfo/Output/attr-btf_type_tag.ll.script: line 2: 35783104 IOT/Abort trap          /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/llc -filetype=obj -o /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/test/DebugInfo/Output/attr-btf_type_tag.ll.tmp /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/DebugInfo/attr-btf_type_tag.ll
--
********************

@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 19, 2024

Hello @azhan92,

Could you please include the full stack trace?

@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 19, 2024

Ok, I see this failure on the test bot here, and there is no stack trace in the message, sadly.

@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 19, 2024

@azhan92 , tbh I don't have a clue on how to proceed. The switchSection() is called from a myriad of places. I'm 99% sure that the failure is not caused by my changes for debug info classes, as it has nothing to do with ELF sections. Hence, it must be something with the test structure itself, that confuses things specifically on AIX. The options are:

  • find someone with access to the AIX machine to at-least get the full stack trace or use llvm-reduce to narrow down the issue;
  • mark the test as x86 only, simply to avoid running it on AIX (kinda bad);
  • revert the whole commit.

Do you happen to know someone with access to AIX?

@nikic
Copy link
Contributor

nikic commented Jun 20, 2024

@eddyz87 You can reproduce the crash just by adding -mtriple=powerpc64-aix to the test. It doesn't require an AIX host.

Possibly the test just needs an explicit triple -- given that this is BPF functionality, it doesn't sound relevant to AIX.

@nikic
Copy link
Contributor

nikic commented Jun 20, 2024

@nikic , you mean RelWithDebInfo or Release w/o additional flags, right? Could you please point me to the script that builds LLVM on the test bot, for some reason I can't figure out which part of the llvm-compile-time-tracker does it...

This is the cmake config: https://github.com/nikic/llvm-compile-time-tracker/blob/master/cmake_llvm_project_stage1.sh

@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 20, 2024

@eddyz87 You can reproduce the crash just by adding -mtriple=powerpc64-aix to the test. It doesn't require an AIX host.

Possibly the test just needs an explicit triple -- given that this is BPF functionality, it doesn't sound relevant to AIX.

Thank you for this suggestion. The test is for BPF functionality, but the expectation is that the relevant debug information would be generated for non-BPF binaries (linux kernel). However, looking at the reasons for the failure (below, under spoiler) I think that you are right, and it is better to hide the test behind REQUIRES: x86-registered-target and -mtriple=x86_64-unknown-linux-gnu. Will submit a PR shortly. Please let me know if you'd like to see revert+re-apply instead.

Test failure details The test fails with the following backtrace:
$ llc -mtriple=powerpc64-aix -filetype=obj -o /dev/null llvm/test/DebugInfo/attr-btf_type_tag.ll
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: llc -mtriple=powerpc64-aix -filetype=obj -o /dev/null /home/eddy/work/llvm-project/llvm/test/DebugInfo/attr-btf_type_tag.ll
 ...
 #4 0x000064cbfb6fd87b llvm::MCSection::getBeginSymbol()
                       /home/eddy/work/llvm-project/llvm/include/llvm/MC/MCSection.h:141:39
 #5 0x000064cbfb6fd87b llvm::DwarfUnit::addStringOffsetsStart()
                       /home/eddy/work/llvm-project/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1885:49
 #6 0x00005e3e18ba72e4 llvm::DwarfDebug::finishUnitAttributes(llvm::DICompileUnit const*, llvm::DwarfCompileUnit&)
                       /home/eddy/work/llvm-project/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1037:11
...

Relevant source code:

// DwarfUnit.cpp
  1879	void DwarfUnit::addStringOffsetsStart() {
  1880	  const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
  1881	  addSectionLabel(getUnitDie(), dwarf::DW_AT_str_offsets_base,
  1882	                  DU->getStringOffsetsStartSym(),
  1883	                  TLOF.getDwarfStrOffSection()->getBeginSymbol());
  1884	}

// DwarfDebug.cpp
   330  DwarfDebug::DwarfDebug(AsmPrinter *A)
   ...
   420    UseSegmentedStringOffsetsTable = DwarfVersion >= 5;
  ...
  1009  void DwarfDebug::finishUnitAttributes(const DICompileUnit *DIUnit,
  1010                                        DwarfCompileUnit &NewCU) {
  ...
  1033      // Add DW_str_offsets_base to the unit DIE, except for split units.
  1034      if (useSegmentedStringOffsetsTable())
  1035        NewCU.addStringOffsetsStart();

Adding some debug prints shows that TLOF.getDwarfStrOffSection() is NULL.
Object file format for powerpc-aix is XCOFF, inspecting MCObjectFileInfo::initCOFFMCObjectFileInfo shows that DwarfStrOffSection is indeed not initialized for this object file format.
Also, using clang to generate ll file for the test from scratch shows that for powerpc-aix DWARF v3 is used instead of v5.

Hence, the following line in the ll file triggers test failure:

!31 = !{i32 7, !"Dwarf Version", i32 5}

eddyz87 added a commit to eddyz87/llvm-project that referenced this pull request Jun 20, 2024
…SubroutineType (llvm#91422)"

This reverts commit 3ca1744.

As reported in [1,2] the commit above causes CI failure for
powerpc-aix target.
There is also a performance regression reported in [3].
Reverting to comply with the developer policy.

[1] llvm#91422 (comment)
[2] https://lab.llvm.org/buildbot/#/builders/64/builds/62
[3] llvm#91422 (comment)
@eddyz87
Copy link
Contributor Author

eddyz87 commented Jun 20, 2024

@nikic , @azhan92, given the outstanding performance issue I opted for a revert:
#96172

@azhan92
Copy link
Contributor

azhan92 commented Jun 20, 2024

@eddyz87 Thanks!

eddyz87 added a commit that referenced this pull request Jun 20, 2024
#96172)

…SubroutineType (#91422)"

This reverts commit 3ca1744.

As reported in [1,2] the commit above causes CI failure for powerpc-aix
target.
There is also a performance regression reported in [3]. Reverting to
comply with the developer policy.

[1]
#91422 (comment)
[2] https://lab.llvm.org/buildbot/#/builders/64/builds/62
[3]
#91422 (comment)
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
llvm#96172)

…SubroutineType (llvm#91422)"

This reverts commit 3ca1744.

As reported in [1,2] the commit above causes CI failure for powerpc-aix
target.
There is also a performance regression reported in [3]. Reverting to
comply with the developer policy.

[1]
llvm#91422 (comment)
[2] https://lab.llvm.org/buildbot/#/builders/64/builds/62
[3]
llvm#91422 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants