[LLVM][DebugInfo] Allow ExtraData field to be a node reference #165023
Conversation
|
@llvm/pr-subscribers-debuginfo Author: Laxman Sole (laxmansole) ChangesThis change enhances debug info metadata handling to support node references in the Full diff: https://github.com/llvm/llvm-project/pull/165023.diff 2 Files Affected:
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index e30df88e6b56b..0113b02547c43 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -959,16 +959,29 @@ DIType *DIDerivedType::getClassType() const {
assert(getTag() == dwarf::DW_TAG_ptr_to_member_type);
return cast_or_null<DIType>(getExtraData());
}
+
+// Helper function to extract ConstantAsMetadata from ExtraData,
+// handling extra data MDTuple unwrapping if needed.
+static ConstantAsMetadata *extractConstantMetadata(Metadata *ExtraData) {
+ Metadata *ED = ExtraData;
+ while (auto *Tuple = dyn_cast_or_null<MDTuple>(ED)) {
+ if (Tuple->getNumOperands() != 1)
+ return nullptr;
+ ED = Tuple->getOperand(0);
+ }
+ return cast_or_null<ConstantAsMetadata>(ED);
+}
+
uint32_t DIDerivedType::getVBPtrOffset() const {
assert(getTag() == dwarf::DW_TAG_inheritance);
- if (auto *CM = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *CM = extractConstantMetadata(getExtraData()))
if (auto *CI = dyn_cast_or_null<ConstantInt>(CM->getValue()))
return static_cast<uint32_t>(CI->getZExtValue());
return 0;
}
Constant *DIDerivedType::getStorageOffsetInBits() const {
assert(getTag() == dwarf::DW_TAG_member && isBitField());
- if (auto *C = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *C = extractConstantMetadata(getExtraData()))
return C->getValue();
return nullptr;
}
@@ -977,13 +990,13 @@ Constant *DIDerivedType::getConstant() const {
assert((getTag() == dwarf::DW_TAG_member ||
getTag() == dwarf::DW_TAG_variable) &&
isStaticMember());
- if (auto *C = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *C = extractConstantMetadata(getExtraData()))
return C->getValue();
return nullptr;
}
Constant *DIDerivedType::getDiscriminantValue() const {
assert(getTag() == dwarf::DW_TAG_member && !isStaticMember());
- if (auto *C = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *C = extractConstantMetadata(getExtraData()))
return C->getValue();
return nullptr;
}
diff --git a/llvm/test/DebugInfo/extradata-node-reference.ll b/llvm/test/DebugInfo/extradata-node-reference.ll
new file mode 100644
index 0000000000000..03240bf9920d5
--- /dev/null
+++ b/llvm/test/DebugInfo/extradata-node-reference.ll
@@ -0,0 +1,91 @@
+;; Test verifies that node reference in the extraData field are handled correctly
+;; when used with tags like DW_TAG_member, DW_TAG_inheritance etc.
+
+; REQUIRES: object-emission
+; RUN: %llc_dwarf %s -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
+
+; Example 1: BitField with storage offset (extraData: i64 0)
+%struct.BitField = type { i8 }
+@bf = global %struct.BitField zeroinitializer, !dbg !9
+
+; Example 2: Static member with constant value (extraData: i32 42)
+%struct.Static = type { i32 }
+@st = global %struct.Static zeroinitializer, !dbg !16
+
+; Example 3: Discriminant value for variant (extraData: i32 100)
+%union.Variant = type { [8 x i8] }
+@var = global %union.Variant zeroinitializer, !dbg !24
+
+; Example 4: Inheritance VBPtr offset (extraData: i32 0)
+%class.Derived = type { i32 }
+@der = global %class.Derived zeroinitializer, !dbg !35
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 11.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !8)
+!1 = !DIFile(filename: "test.cpp", directory: ".")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !{i32 1, !"wchar_size", i32 4}
+!4 = !{i32 2, !"Dwarf Version", i32 5}
+!8 = !{!9, !16, !24, !35}
+
+; extraData node definitions
+!15 = !{i64 0} ; BitField storage offset
+!22 = !{i32 42} ; Static member constant value
+!33 = !{!42}
+!41 = !{i32 0} ; VBPtr offset
+!42 = !{i32 100} ; Discriminant value
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("bf")
+; CHECK: {{.*}} DW_TAG_member
+; CHECK: {{.*}} DW_AT_name ("field")
+; === BitField: extraData holds storage offset ===
+!9 = !DIGlobalVariableExpression(var: !10, expr: !DIExpression())
+!10 = distinct !DIGlobalVariable(name: "bf", scope: !0, file: !1, line: 5, type: !11, isLocal: false, isDefinition: true)
+!11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "BitField", file: !1, line: 5, size: 8, elements: !12)
+!12 = !{!13}
+!13 = !DIDerivedType(tag: DW_TAG_member, name: "field", scope: !11, file: !1, line: 6, baseType: !14, size: 3, flags: DIFlagBitField, extraData: !15)
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("st")
+; CHECK: {{.*}} DW_TAG_member
+; CHECK: {{.*}} DW_AT_name ("const_val")
+; CHECK: {{.*}} DW_AT_const_value (42)
+; === Static Member: extraData holds constant value ===
+!16 = !DIGlobalVariableExpression(var: !17, expr: !DIExpression())
+!17 = distinct !DIGlobalVariable(name: "st", scope: !0, file: !1, line: 10, type: !18, isLocal: false, isDefinition: true)
+!18 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Static", file: !1, line: 10, size: 32, elements: !19)
+!19 = !{!20}
+!20 = !DIDerivedType(tag: DW_TAG_member, name: "const_val", scope: !18, file: !1, line: 11, baseType: !21, flags: DIFlagStaticMember, extraData: !22)
+!21 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !14)
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("var")
+; CHECK: {{.*}} DW_TAG_member
+; CHECK: {{.*}} DW_AT_name ("variant_none")
+; CHECK: {{.*}} DW_AT_discr_value (0x64)
+; === Discriminant: extraData holds discriminant value ===
+!24 = !DIGlobalVariableExpression(var: !25, expr: !DIExpression())
+!25 = distinct !DIGlobalVariable(name: "var", scope: !0, file: !1, line: 15, type: !26, isLocal: false, isDefinition: true)
+!26 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Variant", file: !1, line: 15, size: 128, elements: !27)
+!27 = !{!28}
+!28 = !DICompositeType(tag: DW_TAG_variant_part, scope: !26, file: !1, size: 128, elements: !29, discriminator: !30)
+!29 = !{!31, !32}
+!30 = !DIDerivedType(tag: DW_TAG_member, scope: !28, file: !1, baseType: !14, size: 32, align: 32, flags: DIFlagArtificial)
+!31 = !DIDerivedType(tag: DW_TAG_member, name: "variant_none", scope: !28, file: !1, baseType: !14, size: 32)
+!32 = !DIDerivedType(tag: DW_TAG_member, name: "variant_some", scope: !28, file: !1, baseType: !14, size: 32, extraData: !33)
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("der")
+; CHECK: {{.*}} DW_TAG_inheritance
+; CHECK: {{.*}} DW_AT_type ({{.*}} "Base")
+; === Inheritance: extraData holds VBPtr offset ===
+!35 = !DIGlobalVariableExpression(var: !36, expr: !DIExpression())
+!36 = distinct !DIGlobalVariable(name: "der", scope: !0, file: !1, line: 20, type: !37, isLocal: false, isDefinition: true)
+!37 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Derived", file: !1, line: 20, size: 32, elements: !38)
+!38 = !{!39}
+!39 = !DIDerivedType(tag: DW_TAG_inheritance, scope: !37, baseType: !40, extraData: !41)
+!40 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Base", file: !1, line: 19, size: 32)
|
|
@llvm/pr-subscribers-llvm-ir Author: Laxman Sole (laxmansole) ChangesThis change enhances debug info metadata handling to support node references in the Full diff: https://github.com/llvm/llvm-project/pull/165023.diff 2 Files Affected:
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index e30df88e6b56b..0113b02547c43 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -959,16 +959,29 @@ DIType *DIDerivedType::getClassType() const {
assert(getTag() == dwarf::DW_TAG_ptr_to_member_type);
return cast_or_null<DIType>(getExtraData());
}
+
+// Helper function to extract ConstantAsMetadata from ExtraData,
+// handling extra data MDTuple unwrapping if needed.
+static ConstantAsMetadata *extractConstantMetadata(Metadata *ExtraData) {
+ Metadata *ED = ExtraData;
+ while (auto *Tuple = dyn_cast_or_null<MDTuple>(ED)) {
+ if (Tuple->getNumOperands() != 1)
+ return nullptr;
+ ED = Tuple->getOperand(0);
+ }
+ return cast_or_null<ConstantAsMetadata>(ED);
+}
+
uint32_t DIDerivedType::getVBPtrOffset() const {
assert(getTag() == dwarf::DW_TAG_inheritance);
- if (auto *CM = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *CM = extractConstantMetadata(getExtraData()))
if (auto *CI = dyn_cast_or_null<ConstantInt>(CM->getValue()))
return static_cast<uint32_t>(CI->getZExtValue());
return 0;
}
Constant *DIDerivedType::getStorageOffsetInBits() const {
assert(getTag() == dwarf::DW_TAG_member && isBitField());
- if (auto *C = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *C = extractConstantMetadata(getExtraData()))
return C->getValue();
return nullptr;
}
@@ -977,13 +990,13 @@ Constant *DIDerivedType::getConstant() const {
assert((getTag() == dwarf::DW_TAG_member ||
getTag() == dwarf::DW_TAG_variable) &&
isStaticMember());
- if (auto *C = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *C = extractConstantMetadata(getExtraData()))
return C->getValue();
return nullptr;
}
Constant *DIDerivedType::getDiscriminantValue() const {
assert(getTag() == dwarf::DW_TAG_member && !isStaticMember());
- if (auto *C = cast_or_null<ConstantAsMetadata>(getExtraData()))
+ if (auto *C = extractConstantMetadata(getExtraData()))
return C->getValue();
return nullptr;
}
diff --git a/llvm/test/DebugInfo/extradata-node-reference.ll b/llvm/test/DebugInfo/extradata-node-reference.ll
new file mode 100644
index 0000000000000..03240bf9920d5
--- /dev/null
+++ b/llvm/test/DebugInfo/extradata-node-reference.ll
@@ -0,0 +1,91 @@
+;; Test verifies that node reference in the extraData field are handled correctly
+;; when used with tags like DW_TAG_member, DW_TAG_inheritance etc.
+
+; REQUIRES: object-emission
+; RUN: %llc_dwarf %s -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
+
+; Example 1: BitField with storage offset (extraData: i64 0)
+%struct.BitField = type { i8 }
+@bf = global %struct.BitField zeroinitializer, !dbg !9
+
+; Example 2: Static member with constant value (extraData: i32 42)
+%struct.Static = type { i32 }
+@st = global %struct.Static zeroinitializer, !dbg !16
+
+; Example 3: Discriminant value for variant (extraData: i32 100)
+%union.Variant = type { [8 x i8] }
+@var = global %union.Variant zeroinitializer, !dbg !24
+
+; Example 4: Inheritance VBPtr offset (extraData: i32 0)
+%class.Derived = type { i32 }
+@der = global %class.Derived zeroinitializer, !dbg !35
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 11.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !8)
+!1 = !DIFile(filename: "test.cpp", directory: ".")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !{i32 1, !"wchar_size", i32 4}
+!4 = !{i32 2, !"Dwarf Version", i32 5}
+!8 = !{!9, !16, !24, !35}
+
+; extraData node definitions
+!15 = !{i64 0} ; BitField storage offset
+!22 = !{i32 42} ; Static member constant value
+!33 = !{!42}
+!41 = !{i32 0} ; VBPtr offset
+!42 = !{i32 100} ; Discriminant value
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("bf")
+; CHECK: {{.*}} DW_TAG_member
+; CHECK: {{.*}} DW_AT_name ("field")
+; === BitField: extraData holds storage offset ===
+!9 = !DIGlobalVariableExpression(var: !10, expr: !DIExpression())
+!10 = distinct !DIGlobalVariable(name: "bf", scope: !0, file: !1, line: 5, type: !11, isLocal: false, isDefinition: true)
+!11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "BitField", file: !1, line: 5, size: 8, elements: !12)
+!12 = !{!13}
+!13 = !DIDerivedType(tag: DW_TAG_member, name: "field", scope: !11, file: !1, line: 6, baseType: !14, size: 3, flags: DIFlagBitField, extraData: !15)
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("st")
+; CHECK: {{.*}} DW_TAG_member
+; CHECK: {{.*}} DW_AT_name ("const_val")
+; CHECK: {{.*}} DW_AT_const_value (42)
+; === Static Member: extraData holds constant value ===
+!16 = !DIGlobalVariableExpression(var: !17, expr: !DIExpression())
+!17 = distinct !DIGlobalVariable(name: "st", scope: !0, file: !1, line: 10, type: !18, isLocal: false, isDefinition: true)
+!18 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Static", file: !1, line: 10, size: 32, elements: !19)
+!19 = !{!20}
+!20 = !DIDerivedType(tag: DW_TAG_member, name: "const_val", scope: !18, file: !1, line: 11, baseType: !21, flags: DIFlagStaticMember, extraData: !22)
+!21 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !14)
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("var")
+; CHECK: {{.*}} DW_TAG_member
+; CHECK: {{.*}} DW_AT_name ("variant_none")
+; CHECK: {{.*}} DW_AT_discr_value (0x64)
+; === Discriminant: extraData holds discriminant value ===
+!24 = !DIGlobalVariableExpression(var: !25, expr: !DIExpression())
+!25 = distinct !DIGlobalVariable(name: "var", scope: !0, file: !1, line: 15, type: !26, isLocal: false, isDefinition: true)
+!26 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Variant", file: !1, line: 15, size: 128, elements: !27)
+!27 = !{!28}
+!28 = !DICompositeType(tag: DW_TAG_variant_part, scope: !26, file: !1, size: 128, elements: !29, discriminator: !30)
+!29 = !{!31, !32}
+!30 = !DIDerivedType(tag: DW_TAG_member, scope: !28, file: !1, baseType: !14, size: 32, align: 32, flags: DIFlagArtificial)
+!31 = !DIDerivedType(tag: DW_TAG_member, name: "variant_none", scope: !28, file: !1, baseType: !14, size: 32)
+!32 = !DIDerivedType(tag: DW_TAG_member, name: "variant_some", scope: !28, file: !1, baseType: !14, size: 32, extraData: !33)
+
+; CHECK: {{.*}} DW_TAG_variable
+; CHECK: {{.*}} DW_AT_name ("der")
+; CHECK: {{.*}} DW_TAG_inheritance
+; CHECK: {{.*}} DW_AT_type ({{.*}} "Base")
+; === Inheritance: extraData holds VBPtr offset ===
+!35 = !DIGlobalVariableExpression(var: !36, expr: !DIExpression())
+!36 = distinct !DIGlobalVariable(name: "der", scope: !0, file: !1, line: 20, type: !37, isLocal: false, isDefinition: true)
+!37 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Derived", file: !1, line: 20, size: 32, elements: !38)
+!38 = !{!39}
+!39 = !DIDerivedType(tag: DW_TAG_inheritance, scope: !37, baseType: !40, extraData: !41)
+!40 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Base", file: !1, line: 19, size: 32)
|
There was a problem hiding this comment.
Pull Request Overview
This PR enables the extraData field in debug info metadata to accept both direct constant values and node references (wrapped in MDTuples). This change affects metadata tags DW_TAG_member, DW_TAG_variable, and DW_TAG_inheritance, allowing more flexible metadata representation.
Key Changes:
- Added
extractConstantMetadatahelper function to unwrap MDTuple nodes containing constants - Updated four existing methods to use the new helper instead of direct casts
- Added comprehensive test coverage for all affected use cases
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| llvm/lib/IR/DebugInfoMetadata.cpp | Introduces helper function to unwrap MDTuple references and updates extraData accessors to support both direct constants and node references |
| llvm/test/DebugInfo/extradata-node-reference.ll | Adds test cases verifying correct handling of node references in extraData for bitfields, static members, discriminants, and inheritance |
| static ConstantAsMetadata *extractConstantMetadata(Metadata *ExtraData) { | ||
| Metadata *ED = ExtraData; | ||
| while (auto *Tuple = dyn_cast_or_null<MDTuple>(ED)) { | ||
| if (Tuple->getNumOperands() != 1) | ||
| return nullptr; | ||
| ED = Tuple->getOperand(0); | ||
| } | ||
| return cast_or_null<ConstantAsMetadata>(ED); | ||
| } |
There was a problem hiding this comment.
The while loop can create an infinite loop if an MDTuple contains a reference to itself or creates a cycle. Consider adding cycle detection or a maximum depth limit to prevent infinite recursion through nested tuple structures.
| @@ -0,0 +1,91 @@ | |||
| ;; Test verifies that node reference in the extraData field are handled correctly | |||
There was a problem hiding this comment.
Corrected grammar: 'node reference' should be 'node references' to match plural usage.
| ;; Test verifies that node reference in the extraData field are handled correctly | |
| ;; Test verifies that node references in the extraData field are handled correctly |
This is used to support numba-cuda debugging effort: |
|
I am unfamiliar with numba-cuda, can you explain how the metadata node references will be used? An example could be useful |
Let’s consider the following two cases. Both are valid LLVM IR examples; however, the second case triggers an assertion because the backend lacks proper handling (which this PR addresses).
|
|
I meant an example of how it can be utilised or what it enables in the future. Why is the second form more useful than the first? It looks like both forms are semantically identical, am I missing something? |
|
Yes, both are semantically identical.
Even with the fix in These changes are necessary to support the second form. The rationale is that valid LLVM IR should never trigger assertions. |
|
I feel like I'm being a bit of a stick in the mud here possibly unnecessarily - I'd be perfectly happy if someone else wants to approve this as is. It feels like an odd change to me because although yes it is asserting, which isn't very user-friendly, it seems like accepting non-constant data is not intended. Without knowing the tool at all, naively, it sounds like this would ideally be solved in llvmlite (i.e., emit/consume the metadata constants) rather than in LLVM. Taking this to an extreme, we wouldn't want to accept metadata tuples of constants everywhere that metadata constants are accepted. That said, this area does seem poorly specified. The langref only briefly mentions the extraData field and the verifier doesn't complain. I think in an ideal world the verifier would have complained about the tuple extraData, rather than hitting an assert later. I think I'd feel slightly better about it if verifier checks could be added? |
Yes you are right, this area of specification could use a bit more clarity. I believe the issue is only in DwarfUnit.cpp: To limit the scope one possibility is
|
jryans
changed the title
Verifier didn't complain about the tuple llvm-project/llvm/test/DebugInfo/Generic/debug-info-qualifiers.ll Lines 86 to 93 in 3d42b48 |
laxmansole
force-pushed
the
user/lsole/extraData_node_ref
branch
from
1c2088c to
06e0661
Compare
But !4 in the given example isn't a tuple, is it? Is there something that prevents us from adding such check? |
My bad— I confused llvm-project/llvm/test/DebugInfo/X86/template-alias.ll Lines 58 to 66 in c87e3c9 We'll need to handle the verifier checks separately for different tags and also add constraints in this case, such as preventing nested tuples. If we decide to add those, we can include them in a separate follow-up PR. |
6 participants
This change enhances debug info metadata handling to support node references in the
extraDatafield forDW_TAG_member,DW_TAG_variable, andDW_TAG_inheritancetags.The change enables LLVM to handle both direct constant values (e.g., extraData: i8 1) and node references (e.g., extraData: !18 where !18 = !{ i8 1 }).