Emit DW_AT_object_pointer once, on the declaration, for each function.

This effectively reverts r164326, but adds some comments and
justification and ensures we /don't/ emit the DW_AT_object_pointer on
the (abstract and concrete) definitions. (while still preserving it on
standalone definitions involving ObjC Blocks)

This does increase the size of member function declarations from 7 to 11
bytes, unfortunately, but still seems like the Right Thing to do so that
callers that see only the declaration still have the information about
the object pointer. That said, I don't know what, if any, DWARF
consumers don't have a heuristic to guess this in the case of normal
C++ member functions - perhaps we can remove it entirely.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@207705 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -548,11 +548,16 @@ DIE *DwarfDebug::createScopeChildrenDIE(
 }
 
 void DwarfDebug::createAndAddScopeChildren(DwarfCompileUnit &TheCU,
-                                           LexicalScope *Scope, DIE &ScopeDIE) {
+                                           LexicalScope *Scope,
+                                           DISubprogram Sub, DIE &ScopeDIE) {
   // We create children when the scope DIE is not null.
   SmallVector<std::unique_ptr<DIE>, 8> Children;
   if (DIE *ObjectPointer = createScopeChildrenDIE(TheCU, Scope, Children))
-    TheCU.addDIEEntry(ScopeDIE, dwarf::DW_AT_object_pointer, *ObjectPointer);
+    // The declaration will have the object_pointer, otherwise put it on the
+    // definition. This happens with ObjC blocks that have object_pointer on
+    // non-member functions.
+    if (!Sub.getFunctionDeclaration())
+      TheCU.addDIEEntry(ScopeDIE, dwarf::DW_AT_object_pointer, *ObjectPointer);
 
   // Add children
   for (auto &I : Children)
@@ -571,7 +576,7 @@ void DwarfDebug::constructAbstractSubprogramScopeDIE(DwarfCompileUnit &TheCU,
 
   if (DIE *ScopeDIE = TheCU.getDIE(Sub)) {
     AbstractSPDies.insert(std::make_pair(Sub, ScopeDIE));
-    createAndAddScopeChildren(TheCU, Scope, *ScopeDIE);
+    createAndAddScopeChildren(TheCU, Scope, Sub, *ScopeDIE);
   }
 }
 
@@ -588,7 +593,7 @@ DIE &DwarfDebug::constructSubprogramScopeDIE(DwarfCompileUnit &TheCU,
 
   DIE &ScopeDIE = updateSubprogramScopeDIE(TheCU, Sub);
 
-  createAndAddScopeChildren(TheCU, Scope, ScopeDIE);
+  createAndAddScopeChildren(TheCU, Scope, Sub, ScopeDIE);
 
   return ScopeDIE;
 }

lib/CodeGen/AsmPrinter/DwarfDebug.h

@@ -371,7 +371,7 @@ class DwarfDebug : public AsmPrinterHandler {
   std::unique_ptr<DIE> constructScopeDIE(DwarfCompileUnit &TheCU,
                                          LexicalScope *Scope);
   void createAndAddScopeChildren(DwarfCompileUnit &TheCU, LexicalScope *Scope,
-                                 DIE &ScopeDIE);
+                                 DISubprogram Sub, DIE &ScopeDIE);
   /// \brief Construct a DIE for this abstract scope.
   void constructAbstractSubprogramScopeDIE(DwarfCompileUnit &TheCU,
                                            LexicalScope *Scope);

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

@@ -1164,7 +1164,8 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, DIDerivedType DTy) {
 }
 
 /// constructSubprogramArguments - Construct function argument DIEs.
-void DwarfUnit::constructSubprogramArguments(DIE &Buffer, DIArray Args) {
+DIE *DwarfUnit::constructSubprogramArguments(DIE &Buffer, DIArray Args) {
+  DIE *ObjectPointer = nullptr;
   for (unsigned i = 1, N = Args.getNumElements(); i < N; ++i) {
     DIDescriptor Ty = Args.getElement(i);
     if (Ty.isUnspecifiedParameter()) {
@@ -1175,8 +1176,11 @@ void DwarfUnit::constructSubprogramArguments(DIE &Buffer, DIArray Args) {
       addType(Arg, DIType(Ty));
       if (DIType(Ty).isArtificial())
         addFlag(Arg, dwarf::DW_AT_artificial);
+      if (DIType(Ty).isObjectPointer())
+        ObjectPointer = &Arg;
     }
   }
+  return ObjectPointer;
 }
 
 /// constructTypeDIE - Construct type DIE from DICompositeType.
@@ -1497,7 +1501,8 @@ DIE *DwarfUnit::getOrCreateSubprogramDIE(DISubprogram SP) {
 
     // Add arguments. Do not add arguments for subprogram definition. They will
     // be handled while processing variables.
-    constructSubprogramArguments(SPDie, Args);
+    if (DIE *ObjectPointer = constructSubprogramArguments(SPDie, Args))
+      addDIEEntry(SPDie, dwarf::DW_AT_object_pointer, *ObjectPointer);
   }
 
   if (SP.isArtificial())

lib/CodeGen/AsmPrinter/DwarfUnit.h

@@ -416,7 +416,7 @@ public:
                                             bool isScopeAbstract);
 
   /// constructSubprogramArguments - Construct function argument DIEs.
-  void constructSubprogramArguments(DIE &Buffer, DIArray Args);
+  DIE *constructSubprogramArguments(DIE &Buffer, DIArray Args);
 
   /// Create a DIE with the given Tag, add the DIE to its parent, and
   /// call insertDIE if MD is not null.

test/DebugInfo/X86/DW_AT_object_pointer.ll

@@ -1,13 +1,28 @@
 ; RUN: llc -mtriple=x86_64-apple-darwin %s -o %t -filetype=obj
 ; RUN: llvm-dwarfdump -debug-dump=info %t | FileCheck %s
 
-; CHECK: DW_TAG_formal_parameter [
+; Emit the DW_TAG_object_pointer on the declaration only, not the definition.
-; CHECK-NOT: ""
+; This seems the most correct thing - the DW_TAG_object_pointer is inherited
+; from the declaration to any (abstract or concrete) definitions and DWARF
+; consumers can use this information for callers that can only see the
+; declaration.
+; That said, it isn't very space efficient - making member function
+; declarations 11 bytes instead of 7 and I'm not sure which tools actually
+; use this information here (ObjC Blocks are a different story & most likely
+; require object_pointer) - perhaps we should omit it entirely for space.
+; No overall space impact study has been performed.
+
+; CHECK: [[DEFINITION:0x[0-9a-f]*]]: DW_TAG_subprogram
+; CHECK-NOT: DW_TAG
+; CHECK-NOT: DW_TAG_object_pointer
 ; CHECK: DW_TAG
+
 ; CHECK: DW_TAG_class_type
+; CHECK-NOT: NULL
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT_name {{.*}} "A"
 ; CHECK: DW_AT_object_pointer [DW_FORM_ref4]     (cu + 0x{{[0-9a-f]*}} => {[[PARAM:0x[0-9a-f]*]]})
 ; CHECK: [[PARAM]]:     DW_TAG_formal_parameter
-; CHECK-NEXT: DW_AT_name [DW_FORM_strp]     ( .debug_str[0x{{[0-9a-f]*}}] = "this")
 
 %class.A = type { i32 }
 

test/DebugInfo/X86/inline-member-function.ll

@@ -20,12 +20,21 @@
 ; CHECK-NOT: DW_AT_object_pointer
 ; CHECK: DW_TAG
 
-; But make sure we emit DW_AT_object_pointer on the abstract definition.
+; But make sure we emit DW_AT_object_pointer on the declaration.
-; CHECK: [[ABSTRACT_ORIGIN]]: DW_TAG_subprogram
+; CHECK: DW_TAG_structure_type
+; CHECK-NEXT: DW_AT_name {{.*}} "foo"
 ; CHECK-NOT: NULL
-; CHECK-NOT: TAG
+; CHECK: [[DECLARATION:0x[0-9a-e]*]]: DW_TAG_subprogram
+; CHECK-NOT: DW_TAG
 ; CHECK: DW_AT_object_pointer
 
+; But don't put it on the abstract definition, either.
+; CHECK: [[ABSTRACT_ORIGIN]]: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT_specification {{.*}}{[[DECLARATION]]}
+; CHECK-NOT: NULL
+; CHECK-NOT: DW_AT_object_pointer
+; CHECK: DW_TAG_formal_parameter
+
 %struct.foo = type { i8 }
 
 @i = global i32 0, align 4