DebugInfo: Add ability to not emit DW_AT_vtable_elem_location for virtual functions.

[pcc]

Summary

A virtual index of -1u indicates that the subprogram's virtual index is
unrepresentable (for example, when using the relative vtable ABI), so do
not emit a DW_AT_vtable_elem_location attribute for it.

[dblaikie]

lib/IR/DIBuilder.cpp

698–699

Is zero not sufficient in this spot (& the other below) - since we still have it conditional on virtuality anyway?

test/Assembler/disubprogram.ll

34–53

What's this test testing? I can't quite see what's new/unique/interesting about it.

test/DebugInfo/Generic/virtual-index.ll

47

Given the verbosity of the IR, we usually put all the CHECK lines up above the IR (& include the C++ source that the IR is derived/generate from for illustrative purposes in a comment above the CHECK lines)

[pcc]

• Address review comments

[pcc]

Duncan wrote:

Judicious use of CHECK-SAME: , ... will let you cherry-pick the interesting fields.

Done.

lib/IR/DIBuilder.cpp

698–699

Almost, but the IR writer output is not conditional on virtuality, so for example if a client used DIBuilder::createFunction to create a subprogram the IR writer's output for that subprogram would include a zero virtualIndex. Not a major point, but it does make the IR writer output a little easier to match against.

test/Assembler/disubprogram.ll

34–53

This is testing the changed functionality in the IR writer.

test/DebugInfo/Generic/virtual-index.ll

47

Done

[dblaikie]

looks good - one piece of optional feedback (make the IR writer consistent with the output selection)

lib/IR/DIBuilder.cpp

698–699

Should we just make the IR writer conditional on virtuality, then? seems oddly inconsistent this way?

[pcc]

lib/IR/DIBuilder.cpp

698–699

I don't have a strong opinion on the matter, but I'd normally favour simplicity in the IR writer. But since the bitcode writer writes out these virtual indices (even for non-virtual functions) in VBR format and -1u has a longer representation than 0, I think that tips the scales in favour of using 0 here and adding the conditional to the IR writer. I'll do that then.

include/llvm/IR/DIBuilder.h

public:
     /// \param LinkageName   Mangled function name.
     /// \param File          File where this variable is defined.
     /// \param LineNo        Line number.
     /// \param Ty            Function type.
     /// \param isLocalToUnit True if this function is not externally visible..
     /// \param isDefinition  True if this is a function definition.
     /// \param Virtuality    Attributes describing virtualness. e.g. pure
     ///                      virtual function.
-    /// \param VTableIndex   Index no of this method in virtual table.
+    /// \param VTableIndex   Index no of this method in virtual table, or -1u if
+    ///                      none or unrepresentable.
     /// \param VTableHolder  Type that holds vtable.
     /// \param Flags         e.g. is this function prototyped or not.
     ///                      This flags are used to emit dwarf attributes.
     /// \param isOptimized   True if optimization is ON.
     /// \param TParams       Function template parameters.
     DISubprogram *
     createMethod(DIScope *Scope, StringRef Name, StringRef LinkageName,
                  DIFile *File, unsigned LineNo, DISubroutineType *Ty,
                  bool isLocalToUnit, bool isDefinition, unsigned Virtuality = 0,
-                 unsigned VTableIndex = 0, DIType *VTableHolder = nullptr,
+                 unsigned VTableIndex = -1u, DIType *VTableHolder = nullptr,
                  unsigned Flags = 0, bool isOptimized = false,
                  DITemplateParameterArray TParams = nullptr);
 
     /// This creates new descriptor for a namespace with the specified
     /// parent scope.
     /// \param Scope       Namespace scope
     /// \param Name        Name of this namespace
     /// \param File        Source file

lib/AsmParser/LLParser.cpp

#define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   OPTIONAL(file, MDField, );                                                   \
   OPTIONAL(line, LineField, );                                                 \
   OPTIONAL(type, MDField, );                                                   \
   OPTIONAL(isLocal, MDBoolField, );                                            \
   OPTIONAL(isDefinition, MDBoolField, (true));                                 \
   OPTIONAL(scopeLine, LineField, );                                            \
   OPTIONAL(containingType, MDField, );                                         \
   OPTIONAL(virtuality, DwarfVirtualityField, );                                \
-  OPTIONAL(virtualIndex, MDUnsignedField, (0, UINT32_MAX));                    \
+  OPTIONAL(virtualIndex, MDUnsignedField, (-1u, UINT32_MAX));                  \
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(isOptimized, MDBoolField, );                                        \
   OPTIONAL(templateParams, MDField, );                                         \
   OPTIONAL(declaration, MDField, );                                            \
   OPTIONAL(variables, MDField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

void DwarfUnit::applySubprogramAttributes(const DISubprogram *SP, DIE &SPDie,
   // return type.
   if (Args.size())
     if (auto Ty = resolve(Args[0]))
       addType(SPDie, Ty);
 
   unsigned VK = SP->getVirtuality();
   if (VK) {
     addUInt(SPDie, dwarf::DW_AT_virtuality, dwarf::DW_FORM_data1, VK);
+    if (SP->getVirtualIndex() != -1u) {
       DIELoc *Block = getDIELoc();
       addUInt(*Block, dwarf::DW_FORM_data1, dwarf::DW_OP_constu);
       addUInt(*Block, dwarf::DW_FORM_udata, SP->getVirtualIndex());
       addBlock(SPDie, dwarf::DW_AT_vtable_elem_location, Block);
+    }
     ContainingTypeMap.insert(
         std::make_pair(&SPDie, resolve(SP->getContainingType())));
   }
 
   if (!SP->isDefinition()) {
     addFlag(SPDie, dwarf::DW_AT_declaration);
 
     // Add arguments. Do not add arguments for subprogram definition. They will

lib/IR/AsmWriter.cpp

static void writeDISubprogram(raw_ostream &Out, const DISubprogram *N,
   Printer.printInt("line", N->getLine());
   Printer.printMetadata("type", N->getRawType());
   Printer.printBool("isLocal", N->isLocalToUnit());
   Printer.printBool("isDefinition", N->isDefinition());
   Printer.printInt("scopeLine", N->getScopeLine());
   Printer.printMetadata("containingType", N->getRawContainingType());
   Printer.printDwarfEnum("virtuality", N->getVirtuality(),
                          dwarf::VirtualityString);
-  Printer.printInt("virtualIndex", N->getVirtualIndex());
+  if (N->getVirtualIndex() != -1u)
+    Printer.printInt("virtualIndex", N->getVirtualIndex(), false);
   Printer.printDIFlags("flags", N->getFlags());
   Printer.printBool("isOptimized", N->isOptimized());
   Printer.printMetadata("templateParams", N->getRawTemplateParams());
   Printer.printMetadata("declaration", N->getRawDeclaration());
   Printer.printMetadata("variables", N->getRawVariables());
   Out << ")";
 }
 

lib/IR/DIBuilder.cpp

DISubprogram *DIBuilder::createFunction(
     DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
     unsigned LineNo, DISubroutineType *Ty, bool isLocalToUnit,
     bool isDefinition, unsigned ScopeLine, unsigned Flags, bool isOptimized,
     DITemplateParameterArray TParams, DISubprogram *Decl) {
   auto *Node =
       getSubprogram(/* IsDistinct = */ isDefinition, VMContext,
                     DIScopeRef::get(getNonCompileUnitScope(Context)), Name,
                     LinkageName, File, LineNo, Ty, isLocalToUnit, isDefinition,
-                    ScopeLine, nullptr, 0, 0, Flags, isOptimized, TParams, Decl,
-                    MDTuple::getTemporary(VMContext, None).release());
+                    ScopeLine, nullptr, 0, -1u, Flags, isOptimized, TParams,
+                    Decl, MDTuple::getTemporary(VMContext, None).release());
 
   if (isDefinition)
     AllSubprograms.push_back(Node);
   trackIfUnresolved(Node);
   return Node;
 }
 
 DISubprogram *DIBuilder::createTempFunctionFwdDecl(
     DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
     unsigned LineNo, DISubroutineType *Ty, bool isLocalToUnit,
     bool isDefinition, unsigned ScopeLine, unsigned Flags, bool isOptimized,
     DITemplateParameterArray TParams, DISubprogram *Decl) {
   return DISubprogram::getTemporary(
              VMContext, DIScopeRef::get(getNonCompileUnitScope(Context)), Name,
              LinkageName, File, LineNo, Ty, isLocalToUnit, isDefinition,
-             ScopeLine, nullptr, 0, 0, Flags, isOptimized, TParams, Decl,
+             ScopeLine, nullptr, 0, -1u, Flags, isOptimized, TParams, Decl,
              nullptr)
       .release();
 }
 
 DISubprogram *
 DIBuilder::createMethod(DIScope *Context, StringRef Name, StringRef LinkageName,
                         DIFile *F, unsigned LineNo, DISubroutineType *Ty,
                         bool isLocalToUnit, bool isDefinition, unsigned VK,

test/Assembler/disubprogram.ll

 ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder %s
 
 ; CHECK: define void @_Z3foov() !dbg !9
 define void @_Z3foov() !dbg !9 {
   ret void
 }
 
-; CHECK: !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9}
-!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9}
+; CHECK: !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11}
+!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11}
 
 !0 = !{null}
 !1 = distinct !DICompositeType(tag: DW_TAG_structure_type)
 !2 = !DIFile(filename: "path/to/file", directory: "/path/to/dir")
 !3 = !DISubroutineType(types: !0)
 !4 = distinct !DICompositeType(tag: DW_TAG_structure_type)
 !5 = distinct !{}
 !6 = distinct !{}
 
 ; CHECK: !7 = distinct !DISubprogram(scope: null, isLocal: false, isDefinition: true, isOptimized: false)
 !7 = distinct !DISubprogram()
 
 ; CHECK: !8 = !DISubprogram(scope: null, isLocal: false, isDefinition: false, isOptimized: false)
 !8 = !DISubprogram(isDefinition: false)
 
 ; CHECK: !9 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1, file: !2, line: 7, type: !3, isLocal: true, isDefinition: true, scopeLine: 8, containingType: !4, virtuality: DW_VIRTUALITY_pure_virtual, virtualIndex: 10, flags: DIFlagPrototyped, isOptimized: true, templateParams: !5, declaration: !8, variables: !6)
 !9 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
                             file: !2, line: 7, type: !3, isLocal: true,
                             isDefinition: true, scopeLine: 8, containingType: !4,
                             virtuality: DW_VIRTUALITY_pure_virtual, virtualIndex: 10,
                             flags: DIFlagPrototyped, isOptimized: true,
                             templateParams: !5, declaration: !8, variables: !6)
 
-!10 = !{i32 1, !"Debug Info Version", i32 3}
-!llvm.module.flags = !{!10}
+; CHECK: !10 = distinct !DISubprogram
+; CHECK-SAME: virtualIndex: 0,
+!10 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
+                            file: !2, line: 7, type: !3, isLocal: true,
+                            isDefinition: true, scopeLine: 8, containingType: !4,
+                            virtuality: DW_VIRTUALITY_pure_virtual, virtualIndex: 0,
+                            flags: DIFlagPrototyped, isOptimized: true,
+                            templateParams: !5, declaration: !8, variables: !6)
+
+; CHECK: !11 = distinct !DISubprogram
+; CHECK-NOT: virtualIndex
+!11 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
+                            file: !2, line: 7, type: !3, isLocal: true,
+                            isDefinition: true, scopeLine: 8, containingType: !4,
+                            virtuality: DW_VIRTUALITY_pure_virtual,
+                            flags: DIFlagPrototyped, isOptimized: true,
+                            templateParams: !5, declaration: !8, variables: !6)
+
+!12 = !{i32 1, !"Debug Info Version", i32 3}
+!llvm.module.flags = !{!12}

test/DebugInfo/Generic/virtual-index.ll

+; REQUIRES: object-emission
+
+; RUN: %llc_dwarf -O0 -filetype=obj < %s > %t
+; RUN: llvm-dwarfdump %t | FileCheck %s
+
+; Generated from the following C++ source code:
+;
+; struct A {
+;   virtual void f();
+;   virtual void g();
+; };
+;
+; void A::f() {}
+; void A::g() {}
+;
+; and manually edited to remove virtualIndex attribute on the A::g subprogram.
+
+; CHECK: DW_TAG_subprogram [
+; CHECK: DW_AT_vtable_elem_location [DW_FORM_exprloc]  (<0x2> 10 00 )
+
+; CHECK: DW_TAG_subprogram [
+; CHECK-NOT: DW_AT_vtable_elem_location
+
+%struct.A = type { i32 (...)** }
+
+@_ZTV1A = unnamed_addr constant [4 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.A*)* @_ZN1A1fEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1gEv to i8*)], align 8
+
+define void @_ZN1A1fEv(%struct.A* %this) unnamed_addr !dbg !18 {
+  ret void
+}
+
+define void @_ZN1A1gEv(%struct.A* %this) unnamed_addr !dbg !19 {
+  ret void
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!20, !21}
+!llvm.ident = !{!22}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, retainedTypes: !3, subprograms: !17)
+!1 = !DIFile(filename: "x", directory: "x")
+!2 = !{}
+!3 = !{!4}
+!4 = !DICompositeType(tag: DW_TAG_structure_type, name: "A", file: !1, line: 1, size: 64, align: 64, elements: !5, vtableHolder: !"_ZTS1A", identifier: "_ZTS1A")
+!5 = !{!6, !12, !16}
+!6 = !DIDerivedType(tag: DW_TAG_member, name: "_vptr$A", scope: !1, file: !1, baseType: !7, size: 64, flags: DIFlagArtificial)
+!7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !8, size: 64)
+!8 = !DIDerivedType(tag: DW_TAG_pointer_type, name: "__vtbl_ptr_type", baseType: !9, size: 64)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!12 = !DISubprogram(name: "f", linkageName: "_ZN1A1fEv", scope: !"_ZTS1A", file: !1, line: 2, type: !13, isLocal: false, isDefinition: false, scopeLine: 2, containingType: !"_ZTS1A", virtuality: DW_VIRTUALITY_virtual, virtualIndex: 0, flags: DIFlagPrototyped, isOptimized: false)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null, !15}
+!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !"_ZTS1A", size: 64, align: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!16 = !DISubprogram(name: "g", linkageName: "_ZN1A1gEv", scope: !"_ZTS1A", file: !1, line: 3, type: !13, isLocal: false, isDefinition: false, scopeLine: 3, containingType: !"_ZTS1A", virtuality: DW_VIRTUALITY_virtual, flags: DIFlagPrototyped, isOptimized: false)
+!17 = !{!18, !19}
+!18 = distinct !DISubprogram(name: "f", linkageName: "_ZN1A1fEv", scope: !"_ZTS1A", file: !1, line: 6, type: !13, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: false, declaration: !12, variables: !2)
+!19 = distinct !DISubprogram(name: "g", linkageName: "_ZN1A1gEv", scope: !"_ZTS1A", file: !1, line: 7, type: !13, isLocal: false, isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized: false, declaration: !16, variables: !2)
+!20 = !{i32 2, !"Dwarf Version", i32 4}
+!21 = !{i32 2, !"Debug Info Version", i32 3}
+!22 = !{!"clang version 3.9.0 (trunk 263469) (llvm/trunk 263156)"}
+!23 = !DILocalVariable(name: "this", arg: 1, scope: !18, type: !24, flags: DIFlagArtificial | DIFlagObjectPointer)
+!24 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !"_ZTS1A", size: 64, align: 64)
+!25 = !DIExpression()
+!26 = !DILocation(line: 0, scope: !18)
+!27 = !DILocation(line: 6, column: 14, scope: !18)
+!28 = !DILocalVariable(name: "this", arg: 1, scope: !19, type: !24, flags: DIFlagArtificial | DIFlagObjectPointer)
+!29 = !DILocation(line: 0, scope: !19)
+!30 = !DILocation(line: 7, column: 14, scope: !19)