Drop debug info for DISubprograms that are not referenced by anything

[aprantl]

Summary

This patch drops the debug info for all DISubprograms that are
(a) not attached to an llvm::Function and
(b) not indirectly reachable via inline scopes from any surviving Function and
(c) not reachable from a type (i.e.: member functions).

Background: I'm currently working on a patch to reverse the pointers between DICompileUnit and DISubprogram (for more info check Duncan's RFC on lazy-loading of debug info metadata http://lists.llvm.org/pipermail/llvm-dev/2016-March/097419.html). The idea is to remove the list of subprograms from DICompileUnit and instead point to the owning compile unit from each DISubprogram. After doing this all DISubprograms fulfilling the above criteria will be implicitly dropped unless we go through an extra effort to preserve them. Since there is not much value in preserving DISubprograms that have been optimized away entirely (as in: not even inlined) I'm proposing to just drop them entirely and save some space.

[aprantl]

rebased patch on ToT.

[dblaikie]

Any particular benefit of doing this before doing the remapping from subprogram->cu? (I guess it means doing some of the test case cleanup first, and the change is pretty small - but we'll want to remove those code changes once the remapping is done, right?)

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

529–530

This is usually just written without the != 0

test/DebugInfo/X86/debug-dead-local-var.ll

14–16

Have you checked the commit history for this? It looks pretty deliberate, and while I suspect even if it was we probably still want to make the change you're making - but it'd be good to check the historic reasoning, etc.

[aprantl]

In D18477#383564, @dblaikie wrote:

Any particular benefit of doing this before doing the remapping from subprogram->cu? (I guess it means doing some of the test case cleanup first, and the change is pretty small - but we'll want to remove those code changes once the remapping is done, right?)

Yes. Doing this first means that the remapping becomes an "NFC" commit (if we disregard the bitcode format).

[aprantl]

• delete the now dead collectDeadVariables().
• drop a redundant != 0.

[aprantl]

Digging further through the commit (and radar) history this feature has been added to support dtrace.
Instead of having everyone pay for this, I think it would be a more appropriate resolution for this to add something like a -gfull option to clang that retains every type in the compile unit.

[aprantl]

Ok, given that the potential issue with dtrace is handled in D18565, are there any other comments on this patch?

[aprantl]

Forgot to remove the collectDeadVariables function declaration.

[aprantl]

Ping.

[dblaikie]

Looks good, thanks - I assume a bunch of this will get simpler once you actually reverse the subprogram<>compile unit mapping? (like the extra check added at DwarfDebug.cpp:530)

Also, do you have any numbers of debug info size changes? I imagine this reduces DWARF size perhaps noticeably for linkonce functions?

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

531–533

Did some formatting get changed here? ^ (Phab shows no diff highlight, but that the forBothCUs doesn't line up with the closing '});'

[aprantl]

Here's some data:

Baseline (compiling llc RelWithDebugAsserts)

find lib-OLD -name '*.o' | xargs dwarfdump --file-stats

  File       symtab            strtab            code data         DWARF debug       STABS debug       other             file
 =========  ================= ================= ================= ================= ================= ================= =======================================
1449525800    1417344   0.10%   5616761   0.39%  26037272   1.80%1349841343  93.12%         0   0.00%  66613080   4.60%
Average sizes:
   1884949       1843   0.10%      7303   0.39%     33858   1.80%   1755320  93.12%         0   0.00%     86622   4.60%

with this patch:

find lib -name '*.o' | xargs dwarfdump --file-stats

 File       symtab            strtab            code data         DWARF debug       STABS debug       other             file
 =========  ================= ================= ================= ================= ================= ================= =======================================
1449515428    1417280   0.10%   5616555   0.39%  26036518   1.80%1349833958  93.12%         0   0.00%  66611117   4.60%
Average sizes:
   1884935       1843   0.10%      7303   0.39%     33857   1.80%   1755310  93.12%         0   0.00%     86620   4.60%

We're saving a hot 7kb of DWARF on llc.

[aprantl]

It turns out that I messed up while rebasing the patch. We save 20kb on llc.

Baseline:

 File       symtab            strtab            code data         DWARF debug       STABS debug       other             file
 =========  ================= ================= ================= ================= ================= ================= =======================================

1449525800    1417344   0.10%   5616761   0.39%  26037272   1.80%1349841343  93.12%         0   0.00%  66613080   4.60%
Average sizes:
   1884949       1843   0.10%      7303   0.39%     33858   1.80%   1755320  93.12%         0   0.00%     86622   4.60%

New:

 File       symtab            strtab            code data         DWARF debug       STABS debug       other             file
 =========  ================= ================= ================= ================= ================= ================= =======================================
1449497856    1417280   0.10%   5616535   0.39%  26035595   1.80%1349819842  93.12%         0   0.00%  66608604   4.60%
Average sizes:
   1884912       1843   0.10%      7303   0.39%     33856   1.80%   1755292  93.12%         0   0.00%     86617   4.60%

[aprantl]

r265876

lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

public:
 
   void constructAbstractSubprogramScopeDIE(LexicalScope *Scope);
 
   /// \brief Construct import_module DIE.
   DIE *constructImportedEntityDIE(const DIImportedEntity *Module);
 
   void finishSubprogramDefinition(const DISubprogram *SP);
 
-  void collectDeadVariables(const DISubprogram *SP);
-
   /// Set the skeleton unit associated with this unit.
   void setSkeleton(DwarfCompileUnit &Skel) { Skeleton = &Skel; }
 
   const MCSymbol *getSectionSym() const {
     assert(Section);
     return Section->getBeginSymbol();
   }
 

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (!D && !includeMinimalInlineScopes())
       // inlined versions during codegen. (except in -gmlt ^ where we want
       // to omit these entirely)
       D = getOrCreateSubprogramDIE(SP);
     if (D)
       // And attach the attributes
       applySubprogramAttributesToDefinition(SP, *D);
   }
 }
-void DwarfCompileUnit::collectDeadVariables(const DISubprogram *SP) {
-  assert(SP && "CU's subprogram list contains a non-subprogram");
-  assert(SP->isDefinition() &&
-         "CU's subprogram list contains a subprogram declaration");
-  auto Variables = SP->getVariables();
-  if (Variables.size() == 0)
-    return;
-
-  DIE *SPDIE = DU->getAbstractSPDies().lookup(SP);
-  if (!SPDIE)
-    SPDIE = getDIE(SP);
-  assert(SPDIE);
-  for (const DILocalVariable *DV : Variables) {
-    DbgVariable NewVar(DV, /* IA */ nullptr, DD);
-    auto VariableDie = constructVariableDIE(NewVar);
-    applyVariableAttributes(NewVar, *VariableDie);
-    SPDIE->addChild(std::move(VariableDie));
-  }
-}
 
 void DwarfCompileUnit::emitHeader(bool UseOffsets) {
   // Don't bother labeling the .dwo unit, as its offset isn't used.
   if (!Skeleton) {
     LabelBegin = Asm->createTempSymbol("cu_begin");
     Asm->OutStreamer->EmitLabel(LabelBegin);
   }
 

lib/CodeGen/AsmPrinter/DwarfDebug.h

class DwarfDebug : public DebugHandlerBase {
   void ensureAbstractVariableIsCreatedIfScoped(InlinedVariable Var,
                                                const MDNode *Scope);
 
   DbgVariable *createConcreteVariable(LexicalScope &Scope, InlinedVariable IV);
 
   /// Construct a DIE for this abstract scope.
   void constructAbstractSubprogramScopeDIE(LexicalScope *Scope);
 
-  /// Collect info for variables that were optimized out.
-  void collectDeadVariables();
-
   void finishVariableDefinitions();
 
   void finishSubprogramDefinitions();
 
   /// Finish off debug information after all functions have been
   /// processed.
   void finalizeModuleInfo();
 

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

if (AbsVar && AbsVar->getDIE()) {
       Unit->addDIEEntry(*VariableDie, dwarf::DW_AT_abstract_origin,
                         *AbsVar->getDIE());
     } else
       Unit->applyVariableAttributes(*Var, *VariableDie);
   }
 }
 
 void DwarfDebug::finishSubprogramDefinitions() {
   for (const auto &P : SPMap)
+    if (ProcessedSPNodes.count(P.first))
       forBothCUs(*P.second, [&](DwarfCompileUnit &CU) {
           CU.finishSubprogramDefinition(cast<DISubprogram>(P.first));
         });
 }
 
-// Collect info for variables that were optimized out.
-void DwarfDebug::collectDeadVariables() {
-  const Module *M = MMI->getModule();
-
-  if (NamedMDNode *CU_Nodes = M->getNamedMetadata("llvm.dbg.cu")) {
-    for (MDNode *N : CU_Nodes->operands()) {
-      auto *TheCU = cast<DICompileUnit>(N);
-      // Construct subprogram DIE and add variables DIEs.
-      DwarfCompileUnit *SPCU =
-          static_cast<DwarfCompileUnit *>(CUMap.lookup(TheCU));
-      assert(SPCU && "Unable to find Compile Unit!");
-      for (auto *SP : TheCU->getSubprograms()) {
-        if (ProcessedSPNodes.count(SP) != 0)
-          continue;
-        SPCU->collectDeadVariables(SP);
-      }
-    }
-  }
-}
-
 void DwarfDebug::finalizeModuleInfo() {
   const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
 
   finishSubprogramDefinitions();
 
   finishVariableDefinitions();
 
-  // Collect info for variables that were optimized out.
-  collectDeadVariables();
-
   // Handle anything that needs to be done on a per-unit basis after
   // all other generation.
   for (const auto &P : CUMap) {
     auto &TheCU = *P.second;
     // Emit DW_AT_containing_type attribute to connect types with their
     // vtable holding type.
     TheCU.constructContainingTypeDIEs();
 
void DwarfDebug::endFunction(const MachineFunction *MF) {
   if (!MMI->hasDebugInfo() || LScopes.empty() ||
       !MF->getFunction()->getSubprogram()) {
     // If we don't have a lexical scope for this function then there will
     // be a hole in the range information. Keep note of this by setting the
     // previously used section to nullptr.
     PrevCU = nullptr;
     CurFn = nullptr;
     DebugHandlerBase::endFunction(MF);
+    // Mark functions with no debug info on any instructions, but a
+    // valid DISubprogram as processed.
+    if (auto *SP = MF->getFunction()->getSubprogram())
+      ProcessedSPNodes.insert(SP);
     return;
   }
 
   // Set DwarfDwarfCompileUnitID in MCContext to default value.
   Asm->OutStreamer->getContext().setDwarfCompileUnitID(0);
 
   LexicalScope *FnScope = LScopes.getCurrentFunctionScope();
   auto *SP = cast<DISubprogram>(FnScope->getScopeNode());

test/DebugInfo/Generic/cross-cu-linkonce-distinct.ll

 ; }
 ; int (*x)(int) = &func;
 ; $ cat b.cpp
 ; inline int func(int i) {
 ;   return i * 2;
 ; }
 ; int (*y)(int) = &func;
 
+; The DISubprogram should show up in compile unit a.
 ; CHECK: DW_TAG_compile_unit
-; CHECK:   DW_TAG_subprogram
 ; CHECK-NOT: DW_TAG
-; CHECK:     DW_AT_name {{.*}} "func"
-; CHECK: DW_TAG_compile_unit
-
-; FIXME: Maybe we should drop the subprogram here - since the function was
-; emitted in one CU, due to linkonce_odr uniquing. We certainly don't emit the
-; subprogram here if the source location for this definition is the same (see
-; test/DebugInfo/cross-cu-linkonce.ll), though it's very easy to tickle that
-; into failing even without duplicating the source as has been done in this
-; case (two cpp files in different directories, including the same header that
-; contains an inline function - clang will produce distinct subprogram metadata
-; that won't deduplicate owing to the file location information containing the
-; directory of the source file even though the file name is absolute, not
-; relative)
+; CHECK:    DW_AT_name {{.*}}"b.cpp"
+; CHECK-NOT: DW_TAG_subprogram
 
-; CHECK: DW_TAG_subprogram
+; CHECK: DW_TAG_compile_unit
+; CHECK-NOT: DW_TAG
+; CHECK:     DW_AT_name {{.*}}"a.cpp"
+; CHECK:     DW_AT_name {{.*}} "func"
 
 @x = global i32 (i32)* @_Z4funci, align 8
 @y = global i32 (i32)* @_Z4funci, align 8
 
 ; Function Attrs: inlinehint nounwind uwtable
 define linkonce_odr i32 @_Z4funci(i32 %i) #0 !dbg !4 {
   %1 = alloca i32, align 4
   store i32 %i, i32* %1, align 4

test/DebugInfo/Generic/nodebug.ll

 ; inline __attribute__((always_inline)) void f1() {
 ;   i = 3;
 ; }
 ;
 ; __attribute__((nodebug)) void f2() {
 ;   f1();
 ; }
 
-; Check that there's only one DW_TAG_subprogram, nothing for the 'f2' function.
-; CHECK: DW_TAG_subprogram
-; CHECK-NOT: DW_TAG
-; CHECK:   DW_AT_name {{.*}} "f1"
+; Check that there's no DW_TAG_subprogram, not even for the 'f2' function.
+; CHECK: DW_TAG_compile_unit
 ; CHECK-NOT: DW_TAG_subprogram
 
 @i = external global i32
 
 ; Function Attrs: uwtable
 define void @_Z2f2v() #0 {
 entry:
   store i32 3, i32* @i, align 4, !dbg !11

test/DebugInfo/X86/debug-dead-local-var.ll

 ; RUN: llc -mtriple=x86_64-linux-gnu %s -filetype=obj -o %t
 ; RUN: llvm-dwarfdump %t | FileCheck %s
 
 ; Reconstruct this via clang and -O2.
 ; static void foo() {
 ;   struct X { int a; int b; } xyz;
 ; }
 
 ; int bar() {
 ;   foo();
 ;   return 1;
 ; }
 
-; Check that we still have the structure type for X even though we're not
-; going to emit a low/high_pc for foo.
-; CHECK: DW_TAG_structure_type
+; Check that don't have the structure type for X since its scope has
+; been optimized away.
+; CHECK-NOT: DW_TAG_structure_type
+; CHECK: DW_TAG_subprogram
+; CHECK: DW_AT_name {{.*}}"bar"
+; CHECK-NOT: DW_TAG_structure_type
 
 ; Function Attrs: nounwind readnone uwtable
 define i32 @bar() #0 !dbg !4 {
 entry:
   ret i32 1, !dbg !21
 }
 
 attributes #0 = { nounwind readnone uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }