Skip to content

Two DWARF variant part improvements #138953

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

Merged
merged 1 commit into from
May 8, 2025
Merged

Conversation

tromey
Copy link
Contributor

@tromey tromey commented May 7, 2025

This patch adds a couple of improvements to the LLVM emission of DWARF variant parts. One of these is desirable for Ada, and the other is required.

Currently, when emitting a discriminant, LLVM follows the precise letter of the DWARF standard, which says:

If the variant part has a discriminant, the discriminant is
represented by a separate debugging information entry which is a
child of the variant part entry.

However, for Ada this does not really make sense. In Ada, the discriminant field exists outside of any variant part, and it makes more sense to emit it separately rather than redundantly emit the field once for each variant part.

This extension was arrived at when this was implemented in GCC, and was accepted for DWARF 6, see:

https://dwarfstd.org/issues/180123.1.html

Here the patch simply lifts this restriction: if the discriminant field was already emitted, it isn't re-emitted. This approach allows the Ada compiler to do what it needs without affecting the Rust output.

Second, this patch extends the discriminant to allow multiple values. This is needed by Ada. Here, I chose to use a ConstantDataArray of pairs of integers, with each pair representing a range, as Ada also allows ranges here. This seemed like a reasonably convenient representation.

@llvmbot
Copy link
Member

llvmbot commented May 7, 2025

@llvm/pr-subscribers-llvm-ir

Author: Tom Tromey (tromey)

Changes

This patch adds a couple of improvements to the LLVM emission of DWARF variant parts. One of these is desirable for Ada, and the other is required.

Currently, when emitting a discriminant, LLVM follows the precise letter of the DWARF standard, which says:

If the variant part has a discriminant, the discriminant is
represented by a separate debugging information entry which is a
child of the variant part entry.

However, for Ada this does not really make sense. In Ada, the discriminant field exists outside of any variant part, and it makes more sense to emit it separately rather than redundantly emit the field once for each variant part.

This extension was arrived at when this was implemented in GCC, and was accepted for DWARF 6, see:

https://dwarfstd.org/issues/180123.1.html

Here the patch simply lifts this restriction: if the discriminant field was already emitted, it isn't re-emitted. This approach allows the Ada compiler to do what it needs without affecting the Rust output.

Second, this patch extends the discriminant to allow multiple values. This is needed by Ada. Here, I chose to use a ConstantDataArray of pairs of integers, with each pair representing a range, as Ada also allows ranges here. This seemed like a reasonably convenient representation.


Full diff: https://github.com/llvm/llvm-project/pull/138953.diff

4 Files Affected:

  • (modified) llvm/include/llvm/IR/DIBuilder.h (+3-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+53-8)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h (+5-1)
  • (added) llvm/test/DebugInfo/Generic/discriminant-member.ll (+74)
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 8e62b810ff147..d293c28f5b450 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -394,7 +394,9 @@ namespace llvm {
     /// \param OffsetInBits Member offset.
     /// \param Flags        Flags to encode member attribute, e.g. private
     /// \param Discriminant The discriminant for this branch; null for
-    ///                     the default branch
+    ///                     the default branch.  This may be a
+    ///                     ConstantDataArray if the variant applies
+    ///                     for multiple discriminants.
     /// \param Ty           Parent type.
     DIDerivedType *createVariantMemberType(DIScope *Scope, StringRef Name,
 					   DIFile *File, unsigned LineNo,
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index bf166cbb043de..b5c8b6d0479d4 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -275,7 +275,7 @@ void DwarfUnit::addSInt(DIEValueList &Die, dwarf::Attribute Attribute,
   addAttribute(Die, Attribute, *Form, DIEInteger(Integer));
 }
 
-void DwarfUnit::addSInt(DIELoc &Die, std::optional<dwarf::Form> Form,
+void DwarfUnit::addSInt(DIEValueList &Die, std::optional<dwarf::Form> Form,
                         int64_t Integer) {
   addSInt(Die, (dwarf::Attribute)0, Form, Integer);
 }
@@ -961,6 +961,43 @@ void DwarfUnit::addAnnotation(DIE &Buffer, DINodeArray Annotations) {
   }
 }
 
+void DwarfUnit::addDiscriminant(DIE &Variant, Constant *Discriminant,
+                                bool IsUnsigned) {
+  if (const auto *CI = dyn_cast_or_null<ConstantInt>(Discriminant)) {
+    addInt(Variant, dwarf::DW_AT_discr_value, CI->getValue(), IsUnsigned);
+  } else if (const auto *CA =
+                 dyn_cast_or_null<ConstantDataArray>(Discriminant)) {
+    // Must have an even number of operands.
+    unsigned NElems = CA->getNumElements();
+    if (NElems % 2 != 0) {
+      return;
+    }
+
+    DIEBlock *Block = new (DIEValueAllocator) DIEBlock;
+
+    const auto AddInt = [&](const APInt &Val) {
+      if (IsUnsigned)
+        addUInt(*Block, dwarf::DW_FORM_udata, Val.getZExtValue());
+      else
+        addSInt(*Block, dwarf::DW_FORM_sdata, Val.getSExtValue());
+    };
+
+    for (unsigned I = 0; I < NElems; I += 2) {
+      const APInt LV = CA->getElementAsAPInt(I);
+      const APInt HV = CA->getElementAsAPInt(I + 1);
+      if (LV == HV) {
+        addUInt(*Block, dwarf::DW_FORM_data1, dwarf::DW_DSC_label);
+        AddInt(LV);
+      } else {
+        addUInt(*Block, dwarf::DW_FORM_data1, dwarf::DW_DSC_range);
+        AddInt(LV);
+        AddInt(HV);
+      }
+    }
+    addBlock(Variant, dwarf::DW_AT_discr_list, Block);
+  }
+}
+
 void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
   // Add name if not anonymous or intermediate type.
   StringRef Name = CTy->getName();
@@ -989,8 +1026,17 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
         //    If the variant part has a discriminant, the discriminant is
         //    represented by a separate debugging information entry which is
         //    a child of the variant part entry.
-        DIE &DiscMember = constructMemberDIE(Buffer, Discriminator);
-        addDIEEntry(Buffer, dwarf::DW_AT_discr, DiscMember);
+        // However, for a language like Ada, this yields a weird
+        // result: a discriminant field would have to be emitted
+        // multiple times, once per variant part.  Instead, this DWARF
+        // restriction was lifted for DWARF 6 (see
+        // https://dwarfstd.org/issues/180123.1.html) and so we allow
+        // this here.
+        DIE *DiscDIE = getDIE(Discriminator);
+        if (DiscDIE == nullptr) {
+          DiscDIE = &constructMemberDIE(Buffer, Discriminator);
+        }
+        addDIEEntry(Buffer, dwarf::DW_AT_discr, *DiscDIE);
       }
     }
 
@@ -1016,10 +1062,9 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
           // When emitting a variant part, wrap each member in
           // DW_TAG_variant.
           DIE &Variant = createAndAddDIE(dwarf::DW_TAG_variant, Buffer);
-          if (const ConstantInt *CI =
-              dyn_cast_or_null<ConstantInt>(DDTy->getDiscriminantValue())) {
-	    addInt(Variant, dwarf::DW_AT_discr_value, CI->getValue(),
-		   DD->isUnsignedDIType(Discriminator->getBaseType()));
+          if (Constant *CI = DDTy->getDiscriminantValue()) {
+            addDiscriminant(Variant, CI,
+                            DD->isUnsignedDIType(Discriminator->getBaseType()));
           }
           constructMemberDIE(Variant, DDTy);
         } else {
@@ -1755,7 +1800,7 @@ void DwarfUnit::constructContainingTypeDIEs() {
 }
 
 DIE &DwarfUnit::constructMemberDIE(DIE &Buffer, const DIDerivedType *DT) {
-  DIE &MemberDie = createAndAddDIE(DT->getTag(), Buffer);
+  DIE &MemberDie = createAndAddDIE(DT->getTag(), Buffer, DT);
   StringRef Name = DT->getName();
   if (!Name.empty())
     addString(MemberDie, dwarf::DW_AT_name, Name);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
index 055d7173daec5..e1156bccfb1ab 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
@@ -165,7 +165,8 @@ class DwarfUnit : public DIEUnit {
   void addSInt(DIEValueList &Die, dwarf::Attribute Attribute,
                std::optional<dwarf::Form> Form, int64_t Integer);
 
-  void addSInt(DIELoc &Die, std::optional<dwarf::Form> Form, int64_t Integer);
+  void addSInt(DIEValueList &Die, std::optional<dwarf::Form> Form,
+               int64_t Integer);
 
   /// Add an integer attribute data and value; value may be any width.
   void addInt(DIE &Die, dwarf::Attribute Attribute, const APInt &Integer,
@@ -342,6 +343,9 @@ class DwarfUnit : public DIEUnit {
   /// form.
   void addIntAsBlock(DIE &Die, dwarf::Attribute Attribute, const APInt &Val);
 
+  // Add discriminant constants to a DW_TAG_variant DIE.
+  void addDiscriminant(DIE &Variant, Constant *Discriminant, bool IsUnsigned);
+
   void constructTypeDIE(DIE &Buffer, const DIBasicType *BTy);
   void constructTypeDIE(DIE &Buffer, const DIFixedPointType *BTy);
   void constructTypeDIE(DIE &Buffer, const DIStringType *BTy);
diff --git a/llvm/test/DebugInfo/Generic/discriminant-member.ll b/llvm/test/DebugInfo/Generic/discriminant-member.ll
new file mode 100644
index 0000000000000..8359aef870e14
--- /dev/null
+++ b/llvm/test/DebugInfo/Generic/discriminant-member.ll
@@ -0,0 +1,74 @@
+; RUN: %llc_dwarf -O0 -filetype=obj < %s > %t
+; RUN: llvm-dwarfdump -v -debug-info %t | FileCheck %s
+
+; Check for a variant part that has two members, where one uses a list
+; of discriminants, and where both refer to a DIE that is not a child
+; of the variant.
+
+; CHECK: DW_AT_name [DW_FORM_strp]  ({{.*}} = "Discr")
+; CHECK: DW_TAG_variant_part
+;   CHECK-NOT: TAG
+;     CHECK: DW_AT_discr [DW_FORM_ref4] (cu + {{0x[0-9a-fA-F]+}} => {[[OFFSET:0x[0-9a-fA-F]+]]})
+;     CHECK: DW_TAG_variant
+;       CHECK: DW_AT_discr_list [DW_FORM_block1] (<0x05> 00 17 01 61 6c )
+;       CHECK: DW_TAG_member
+;         CHECK: DW_AT_name [DW_FORM_strp]  ({{.*}} = "var0")
+;         CHECK: DW_AT_type
+;         CHECK: DW_AT_alignment
+;         CHECK: DW_AT_data_member_location [DW_FORM_data1]	(0x00)
+;     CHECK: DW_TAG_variant
+;       CHECK: DW_AT_discr_value [DW_FORM_data1] (0x4b)
+;       CHECK: DW_TAG_member
+;         CHECK: DW_AT_type
+;         CHECK: DW_AT_alignment
+;         CHECK: DW_AT_data_member_location [DW_FORM_data1]	(0x00)
+
+%F = type { [0 x i8], ptr, [8 x i8] }
+%"F::Nope" = type {}
+
+define internal void @_ZN2e34main17h934ff72f9a38d4bbE() unnamed_addr #0 !dbg !5 {
+start:
+  %qq = alloca %F, align 8
+  call void @llvm.dbg.declare(metadata ptr %qq, metadata !10, metadata !28), !dbg !29
+  store ptr null, ptr %qq, !dbg !29
+  ret void, !dbg !30
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+attributes #0 = { nounwind uwtable }
+
+!llvm.module.flags = !{!0, !1}
+!llvm.dbg.cu = !{!2}
+
+!0 = !{i32 1, !"PIE Level", i32 2}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = distinct !DICompileUnit(language: DW_LANG_Ada95, file: !3, producer: "gnat-llvm", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4)
+!3 = !DIFile(filename: "e3.rs", directory: "/home/tromey/Ada")
+!4 = !{}
+!5 = distinct !DISubprogram(name: "main", linkageName: "_ZN2e34mainE", scope: !6, file: !3, line: 2, type: !8, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagMainSubprogram, isOptimized: false, unit: !2, templateParams: !4, retainedNodes: !4)
+!6 = !DINamespace(name: "e3", scope: null)
+!7 = !DIFile(filename: "<unknown>", directory: "")
+!8 = !DISubroutineType(types: !9)
+!9 = !{null}
+!10 = !DILocalVariable(name: "qq", scope: !11, file: !3, line: 3, type: !12, align: 8)
+!11 = distinct !DILexicalBlock(scope: !5, file: !3, line: 3, column: 4)
+!12 = !DICompositeType(tag: DW_TAG_structure_type, name: "F", scope: !6, file: !7, size: 128, align: 64, elements: !13, identifier: "7ce1efff6b82281ab9ceb730566e7e20")
+!13 = !{!14, !15}
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "Discr", scope: !12, file: !7, baseType: !27, size: 64, align: 64)
+!15 = !DICompositeType(tag: DW_TAG_variant_part, scope: !12, file: !7, size: 128, align: 64, elements: !16, identifier: "7ce1efff6b82281ab9ceb730566e7e20", discriminator: !14)
+!16 = !{!17, !24}
+!17 = !DIDerivedType(tag: DW_TAG_member, name: "var0", scope: !15, file: !7, baseType: !18, size: 128, align: 64, extraData: [4 x i32] [i32 23, i32 23, i32 97, i32 108])
+!18 = !DICompositeType(tag: DW_TAG_structure_type, name: "Yep", scope: !12, file: !7, size: 128, align: 64, elements: !19, identifier: "7ce1efff6b82281ab9ceb730566e7e20::Yep")
+!19 = !{!20, !22}
+!20 = !DIDerivedType(tag: DW_TAG_member, name: "var1", scope: !18, file: !7, baseType: !21, size: 8, align: 8, offset: 64)
+!21 = !DIBasicType(name: "u8", size: 8, encoding: DW_ATE_unsigned)
+!22 = !DIDerivedType(tag: DW_TAG_member, name: "var2", scope: !18, file: !7, baseType: !23, size: 64, align: 64)
+!23 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "&u8", baseType: !21, size: 64, align: 64)
+!24 = !DIDerivedType(tag: DW_TAG_member, scope: !15, file: !7, baseType: !25, size: 128, align: 64, extraData: i32 75)
+!25 = !DICompositeType(tag: DW_TAG_structure_type, name: "Nope", scope: !12, file: !7, size: 128, align: 64, elements: !4, identifier: "7ce1efff6b82281ab9ceb730566e7e20::Nope")
+!27 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)
+!28 = !DIExpression()
+!29 = !DILocation(line: 3, scope: !11)
+!30 = !DILocation(line: 4, scope: !5)

@llvmbot
Copy link
Member

llvmbot commented May 7, 2025

@llvm/pr-subscribers-debuginfo

Author: Tom Tromey (tromey)

Changes

This patch adds a couple of improvements to the LLVM emission of DWARF variant parts. One of these is desirable for Ada, and the other is required.

Currently, when emitting a discriminant, LLVM follows the precise letter of the DWARF standard, which says:

If the variant part has a discriminant, the discriminant is
represented by a separate debugging information entry which is a
child of the variant part entry.

However, for Ada this does not really make sense. In Ada, the discriminant field exists outside of any variant part, and it makes more sense to emit it separately rather than redundantly emit the field once for each variant part.

This extension was arrived at when this was implemented in GCC, and was accepted for DWARF 6, see:

https://dwarfstd.org/issues/180123.1.html

Here the patch simply lifts this restriction: if the discriminant field was already emitted, it isn't re-emitted. This approach allows the Ada compiler to do what it needs without affecting the Rust output.

Second, this patch extends the discriminant to allow multiple values. This is needed by Ada. Here, I chose to use a ConstantDataArray of pairs of integers, with each pair representing a range, as Ada also allows ranges here. This seemed like a reasonably convenient representation.


Full diff: https://github.com/llvm/llvm-project/pull/138953.diff

4 Files Affected:

  • (modified) llvm/include/llvm/IR/DIBuilder.h (+3-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+53-8)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h (+5-1)
  • (added) llvm/test/DebugInfo/Generic/discriminant-member.ll (+74)
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 8e62b810ff147..d293c28f5b450 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -394,7 +394,9 @@ namespace llvm {
     /// \param OffsetInBits Member offset.
     /// \param Flags        Flags to encode member attribute, e.g. private
     /// \param Discriminant The discriminant for this branch; null for
-    ///                     the default branch
+    ///                     the default branch.  This may be a
+    ///                     ConstantDataArray if the variant applies
+    ///                     for multiple discriminants.
     /// \param Ty           Parent type.
     DIDerivedType *createVariantMemberType(DIScope *Scope, StringRef Name,
 					   DIFile *File, unsigned LineNo,
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index bf166cbb043de..b5c8b6d0479d4 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -275,7 +275,7 @@ void DwarfUnit::addSInt(DIEValueList &Die, dwarf::Attribute Attribute,
   addAttribute(Die, Attribute, *Form, DIEInteger(Integer));
 }
 
-void DwarfUnit::addSInt(DIELoc &Die, std::optional<dwarf::Form> Form,
+void DwarfUnit::addSInt(DIEValueList &Die, std::optional<dwarf::Form> Form,
                         int64_t Integer) {
   addSInt(Die, (dwarf::Attribute)0, Form, Integer);
 }
@@ -961,6 +961,43 @@ void DwarfUnit::addAnnotation(DIE &Buffer, DINodeArray Annotations) {
   }
 }
 
+void DwarfUnit::addDiscriminant(DIE &Variant, Constant *Discriminant,
+                                bool IsUnsigned) {
+  if (const auto *CI = dyn_cast_or_null<ConstantInt>(Discriminant)) {
+    addInt(Variant, dwarf::DW_AT_discr_value, CI->getValue(), IsUnsigned);
+  } else if (const auto *CA =
+                 dyn_cast_or_null<ConstantDataArray>(Discriminant)) {
+    // Must have an even number of operands.
+    unsigned NElems = CA->getNumElements();
+    if (NElems % 2 != 0) {
+      return;
+    }
+
+    DIEBlock *Block = new (DIEValueAllocator) DIEBlock;
+
+    const auto AddInt = [&](const APInt &Val) {
+      if (IsUnsigned)
+        addUInt(*Block, dwarf::DW_FORM_udata, Val.getZExtValue());
+      else
+        addSInt(*Block, dwarf::DW_FORM_sdata, Val.getSExtValue());
+    };
+
+    for (unsigned I = 0; I < NElems; I += 2) {
+      const APInt LV = CA->getElementAsAPInt(I);
+      const APInt HV = CA->getElementAsAPInt(I + 1);
+      if (LV == HV) {
+        addUInt(*Block, dwarf::DW_FORM_data1, dwarf::DW_DSC_label);
+        AddInt(LV);
+      } else {
+        addUInt(*Block, dwarf::DW_FORM_data1, dwarf::DW_DSC_range);
+        AddInt(LV);
+        AddInt(HV);
+      }
+    }
+    addBlock(Variant, dwarf::DW_AT_discr_list, Block);
+  }
+}
+
 void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
   // Add name if not anonymous or intermediate type.
   StringRef Name = CTy->getName();
@@ -989,8 +1026,17 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
         //    If the variant part has a discriminant, the discriminant is
         //    represented by a separate debugging information entry which is
         //    a child of the variant part entry.
-        DIE &DiscMember = constructMemberDIE(Buffer, Discriminator);
-        addDIEEntry(Buffer, dwarf::DW_AT_discr, DiscMember);
+        // However, for a language like Ada, this yields a weird
+        // result: a discriminant field would have to be emitted
+        // multiple times, once per variant part.  Instead, this DWARF
+        // restriction was lifted for DWARF 6 (see
+        // https://dwarfstd.org/issues/180123.1.html) and so we allow
+        // this here.
+        DIE *DiscDIE = getDIE(Discriminator);
+        if (DiscDIE == nullptr) {
+          DiscDIE = &constructMemberDIE(Buffer, Discriminator);
+        }
+        addDIEEntry(Buffer, dwarf::DW_AT_discr, *DiscDIE);
       }
     }
 
@@ -1016,10 +1062,9 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
           // When emitting a variant part, wrap each member in
           // DW_TAG_variant.
           DIE &Variant = createAndAddDIE(dwarf::DW_TAG_variant, Buffer);
-          if (const ConstantInt *CI =
-              dyn_cast_or_null<ConstantInt>(DDTy->getDiscriminantValue())) {
-	    addInt(Variant, dwarf::DW_AT_discr_value, CI->getValue(),
-		   DD->isUnsignedDIType(Discriminator->getBaseType()));
+          if (Constant *CI = DDTy->getDiscriminantValue()) {
+            addDiscriminant(Variant, CI,
+                            DD->isUnsignedDIType(Discriminator->getBaseType()));
           }
           constructMemberDIE(Variant, DDTy);
         } else {
@@ -1755,7 +1800,7 @@ void DwarfUnit::constructContainingTypeDIEs() {
 }
 
 DIE &DwarfUnit::constructMemberDIE(DIE &Buffer, const DIDerivedType *DT) {
-  DIE &MemberDie = createAndAddDIE(DT->getTag(), Buffer);
+  DIE &MemberDie = createAndAddDIE(DT->getTag(), Buffer, DT);
   StringRef Name = DT->getName();
   if (!Name.empty())
     addString(MemberDie, dwarf::DW_AT_name, Name);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
index 055d7173daec5..e1156bccfb1ab 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
@@ -165,7 +165,8 @@ class DwarfUnit : public DIEUnit {
   void addSInt(DIEValueList &Die, dwarf::Attribute Attribute,
                std::optional<dwarf::Form> Form, int64_t Integer);
 
-  void addSInt(DIELoc &Die, std::optional<dwarf::Form> Form, int64_t Integer);
+  void addSInt(DIEValueList &Die, std::optional<dwarf::Form> Form,
+               int64_t Integer);
 
   /// Add an integer attribute data and value; value may be any width.
   void addInt(DIE &Die, dwarf::Attribute Attribute, const APInt &Integer,
@@ -342,6 +343,9 @@ class DwarfUnit : public DIEUnit {
   /// form.
   void addIntAsBlock(DIE &Die, dwarf::Attribute Attribute, const APInt &Val);
 
+  // Add discriminant constants to a DW_TAG_variant DIE.
+  void addDiscriminant(DIE &Variant, Constant *Discriminant, bool IsUnsigned);
+
   void constructTypeDIE(DIE &Buffer, const DIBasicType *BTy);
   void constructTypeDIE(DIE &Buffer, const DIFixedPointType *BTy);
   void constructTypeDIE(DIE &Buffer, const DIStringType *BTy);
diff --git a/llvm/test/DebugInfo/Generic/discriminant-member.ll b/llvm/test/DebugInfo/Generic/discriminant-member.ll
new file mode 100644
index 0000000000000..8359aef870e14
--- /dev/null
+++ b/llvm/test/DebugInfo/Generic/discriminant-member.ll
@@ -0,0 +1,74 @@
+; RUN: %llc_dwarf -O0 -filetype=obj < %s > %t
+; RUN: llvm-dwarfdump -v -debug-info %t | FileCheck %s
+
+; Check for a variant part that has two members, where one uses a list
+; of discriminants, and where both refer to a DIE that is not a child
+; of the variant.
+
+; CHECK: DW_AT_name [DW_FORM_strp]  ({{.*}} = "Discr")
+; CHECK: DW_TAG_variant_part
+;   CHECK-NOT: TAG
+;     CHECK: DW_AT_discr [DW_FORM_ref4] (cu + {{0x[0-9a-fA-F]+}} => {[[OFFSET:0x[0-9a-fA-F]+]]})
+;     CHECK: DW_TAG_variant
+;       CHECK: DW_AT_discr_list [DW_FORM_block1] (<0x05> 00 17 01 61 6c )
+;       CHECK: DW_TAG_member
+;         CHECK: DW_AT_name [DW_FORM_strp]  ({{.*}} = "var0")
+;         CHECK: DW_AT_type
+;         CHECK: DW_AT_alignment
+;         CHECK: DW_AT_data_member_location [DW_FORM_data1]	(0x00)
+;     CHECK: DW_TAG_variant
+;       CHECK: DW_AT_discr_value [DW_FORM_data1] (0x4b)
+;       CHECK: DW_TAG_member
+;         CHECK: DW_AT_type
+;         CHECK: DW_AT_alignment
+;         CHECK: DW_AT_data_member_location [DW_FORM_data1]	(0x00)
+
+%F = type { [0 x i8], ptr, [8 x i8] }
+%"F::Nope" = type {}
+
+define internal void @_ZN2e34main17h934ff72f9a38d4bbE() unnamed_addr #0 !dbg !5 {
+start:
+  %qq = alloca %F, align 8
+  call void @llvm.dbg.declare(metadata ptr %qq, metadata !10, metadata !28), !dbg !29
+  store ptr null, ptr %qq, !dbg !29
+  ret void, !dbg !30
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+attributes #0 = { nounwind uwtable }
+
+!llvm.module.flags = !{!0, !1}
+!llvm.dbg.cu = !{!2}
+
+!0 = !{i32 1, !"PIE Level", i32 2}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = distinct !DICompileUnit(language: DW_LANG_Ada95, file: !3, producer: "gnat-llvm", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4)
+!3 = !DIFile(filename: "e3.rs", directory: "/home/tromey/Ada")
+!4 = !{}
+!5 = distinct !DISubprogram(name: "main", linkageName: "_ZN2e34mainE", scope: !6, file: !3, line: 2, type: !8, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagMainSubprogram, isOptimized: false, unit: !2, templateParams: !4, retainedNodes: !4)
+!6 = !DINamespace(name: "e3", scope: null)
+!7 = !DIFile(filename: "<unknown>", directory: "")
+!8 = !DISubroutineType(types: !9)
+!9 = !{null}
+!10 = !DILocalVariable(name: "qq", scope: !11, file: !3, line: 3, type: !12, align: 8)
+!11 = distinct !DILexicalBlock(scope: !5, file: !3, line: 3, column: 4)
+!12 = !DICompositeType(tag: DW_TAG_structure_type, name: "F", scope: !6, file: !7, size: 128, align: 64, elements: !13, identifier: "7ce1efff6b82281ab9ceb730566e7e20")
+!13 = !{!14, !15}
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "Discr", scope: !12, file: !7, baseType: !27, size: 64, align: 64)
+!15 = !DICompositeType(tag: DW_TAG_variant_part, scope: !12, file: !7, size: 128, align: 64, elements: !16, identifier: "7ce1efff6b82281ab9ceb730566e7e20", discriminator: !14)
+!16 = !{!17, !24}
+!17 = !DIDerivedType(tag: DW_TAG_member, name: "var0", scope: !15, file: !7, baseType: !18, size: 128, align: 64, extraData: [4 x i32] [i32 23, i32 23, i32 97, i32 108])
+!18 = !DICompositeType(tag: DW_TAG_structure_type, name: "Yep", scope: !12, file: !7, size: 128, align: 64, elements: !19, identifier: "7ce1efff6b82281ab9ceb730566e7e20::Yep")
+!19 = !{!20, !22}
+!20 = !DIDerivedType(tag: DW_TAG_member, name: "var1", scope: !18, file: !7, baseType: !21, size: 8, align: 8, offset: 64)
+!21 = !DIBasicType(name: "u8", size: 8, encoding: DW_ATE_unsigned)
+!22 = !DIDerivedType(tag: DW_TAG_member, name: "var2", scope: !18, file: !7, baseType: !23, size: 64, align: 64)
+!23 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "&u8", baseType: !21, size: 64, align: 64)
+!24 = !DIDerivedType(tag: DW_TAG_member, scope: !15, file: !7, baseType: !25, size: 128, align: 64, extraData: i32 75)
+!25 = !DICompositeType(tag: DW_TAG_structure_type, name: "Nope", scope: !12, file: !7, size: 128, align: 64, elements: !4, identifier: "7ce1efff6b82281ab9ceb730566e7e20::Nope")
+!27 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)
+!28 = !DIExpression()
+!29 = !DILocation(line: 3, scope: !11)
+!30 = !DILocation(line: 4, scope: !5)

This patch adds a couple of improvements to the LLVM emission of DWARF
variant parts.  One of these is desirable for Ada, and the other is
required.

Currently, when emitting a discriminant, LLVM follows the precise
letter of the DWARF standard, which says:

    If the variant part has a discriminant, the discriminant is
    represented by a separate debugging information entry which is a
    child of the variant part entry.

However, for Ada this does not really make sense.  In Ada, the
discriminant field exists outside of any variant part, and it makes
more sense to emit it separately rather than redundantly emit the
field once for each variant part.

This extension was arrived at when this was implemented in GCC, and
was accepted for DWARF 6, see:

    https://dwarfstd.org/issues/180123.1.html

Here the patch simply lifts this restriction: if the discriminant
field was already emitted, it isn't re-emitted.  This approach allows
the Ada compiler to do what it needs without affecting the Rust
output.

Second, this patch extends the discriminant to allow multiple values.
This is needed by Ada.  Here, I chose to use a ConstantDataArray of
pairs of integers, with each pair representing a range, as Ada also
allows ranges here.  This seemed like a reasonably convenient
representation.
@tromey
Copy link
Contributor Author

tromey commented May 8, 2025

While looking into this code some more today, I found out that more changes will be needed for Ada. So while this patch is necessary, it's not sufficient. Not sure it really matters but I thought I'd point it out. I'll mention it on the discourse thread.

Also I do not have write access, so could someone please land this for me? Thank you.

@dwblaikie dwblaikie merged commit b0bf48d into llvm:main May 8, 2025
9 of 10 checks passed
@tromey tromey deleted the variant-parts branch May 8, 2025 17:09
@hubert-reinterpretcast
Copy link
Collaborator

The newly added test file is failing on the AIX buildbot. @tromey, can you please take a look?
https://lab.llvm.org/buildbot/#/builders/64/builds/3465/steps/6/logs/FAIL__LLVM__discriminant-member_ll

@tromey
Copy link
Contributor Author

tromey commented May 9, 2025

The newly added test file is failing on the AIX buildbot. @tromey, can you please take a look? https://lab.llvm.org/buildbot/#/builders/64/builds/3465/steps/6/logs/FAIL__LLVM__discriminant-member_ll

I'll look at it today. Sorry about the breakage.

@tromey
Copy link
Contributor Author

tromey commented May 9, 2025

Ok, see #139258

hubert-reinterpretcast added a commit that referenced this pull request May 9, 2025
The new test discriminant-member.ll (see #138953) failed on AIX. It
seems that the string form is different in the DWARF. The log reads:

          50:  DW_AT_name [DW_FORM_string] ("Discr")

... but the test only looks for DW_FORM_strp. Since the precise form
isn't important here, this patch changes the test to accept any string
form.

---------

Co-authored-by: Hubert Tong <[email protected]>
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.

4 participants