Module flag to suppress the DWARF entries DW_AT_MIPS_linkage_name and DW_AT_linkage_name.

[wolfgangp]

Summary

I’d like to add a way to suppress DW_AT_MIPS_linkage_name and DW_AT_linkage_name entries in DWARF. Measurements of debug info size (specifically, the .debug_str section) have shown that some decent reductions are to be had by not emitting these entries, benefiting DWARF consumers that do not use them.

[dblaikie]

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

45

I believe we're trying to move away from surfacing command line flags in this way for user-facing features. (see Chris Bieneman's work).

I'm not sure what the right answner is these days - this could be a flag on the compile_unit metadata - in which case it could be properly respected under LTO (if one CU was built with the flag and one built without it, it'd be possible/practical/easy to respect those differences, emitting linkage name for one CU and omitting it in another).

1470

I'd consider sinking the condition down into line 1476 so that we still check the metadata invariants around linkage names, if they're present, even if we're not emitting them.

test/DebugInfo/suppress-linkage-entry-3.ll

4 ↗

(On Diff #14137)

I usually skip the temporary file and pipe the result of llc straight into llvm-dwarfdump (use '-' as the file name for llvm-dwarfdump to read standard input).

test/DebugInfo/suppress-linkage-entry-4.ll

8 ↗

(On Diff #14137)

What's the important distinction between this test and the previous one? It looks like we've only got one codepath so I'm not sure if we need two test cases.

[echristo]

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

45

It's definitely a code gen option and so should go on a code gen option struct. That said, we're probably getting enough of them that a sub struct for debug info would be better.

i.e. I don't see why we'd want to handle adding or subtracting on a CU basis and thus need to put it on the metadata.

[wolfgangp]

Thanks for the comments, David.

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

45

Using a flag on the CU metadata is definitely preferable due to LTO. It also avoids cluttering up the command line, since we want to suppress the attributes unconditionally anyway (without breaking the existing debuginfo tests that check for them). I'll scratch the current change and start over.

test/DebugInfo/suppress-linkage-entry-4.ll

8 ↗

(On Diff #14137)

Right, I was vainly trying to catch future dwarf-3/dwarf-4 deviations, but it doesn't seem worth it.

[echristo]

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

45

I'll be clear, it's not to go onto the CU metadata. There's no reason for it to do so.

[wolfgangp]

Based on everyone's feedback this is the second attempt at providing a way to suppress the DWARF linkage name attributes.

Instead of implementing a backend option, this change implements a metadata module flag “Dwarf Gen Flags”, which itself is intended as a list of flags controlling Dwarf emission. The only supported element of the list is “suppress-linkage-names” which, when present, causes the DWARF attributes DW_AT_MIPS_linkage_name and DW_AT_link_name to be suppressed.

This implementation works also for LTO, although the presence of the flag in one module suppresses the attributes for the entire link, which is different from the non-LTO behavior (only the attributes in the modules with the flag are suppressed).

[wolfgangp]

ping

[wolfgangp]

Revised patch after recent changes to DwarfCompileUnit.

[dblaikie]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

2326

So debug info already has 2 metadata flags (Dwarf Version and Debug Info Version (the version of the metadata schema)), this is a third, and they're all parsed in different places (interestingly the Dwarf Version is parsed in llvm::Module, but the Debug Info Version is parsed in a standalone function (in lib/DebugInfo) llvm::getDebugMetadataVersionFromModule) and this third will be a new place again.

Perhaps these should go somewhere common? Certainly the string constants should be written once (not repeated in every client of LLVM that wants to use them).

test/DebugInfo/suppress-linkage-entries.ll

52

I'm guessing this '6' is a merge specifier? What kind of merging does this designate?

[wolfgangp]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

2326

It certainly seems that way. Being an llvm newbie I'm a bit hesitant to suggest more encompassing changes, but I'd be happy to give it a try.

test/DebugInfo/suppress-linkage-entries.ll

52

It's Module::ModFlagBehavior::AppendUnique, i.e. during a merge values from 2 lists will be appended but duplicates will be dropped.

[wolfgangp]

Abandoned. Functionality implemented with r244678.

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

     }
     // Add the linkage name.
     StringRef LinkageName = GV.getLinkageName();
-    if (!LinkageName.empty())
+    if (DD->emitLinkageNames() && !LinkageName.empty())
       // From DWARF4: DIEs to which DW_AT_linkage_name may apply include:
       // TAG_common_block, TAG_constant, TAG_entry_point, TAG_subprogram and
       // TAG_variable.

 
     // If the linkage name is different than the name, go ahead and output
     // that as well into the name table.
-    if (GV.getLinkageName() != "" && GV.getName() != GV.getLinkageName())
+    if (DD->emitLinkageNames() &&
+        GV.getLinkageName() != "" && GV.getName() != GV.getLinkageName())
       DD->addAccelName(GV.getLinkageName(), *ResultDIE);
   }
 

lib/CodeGen/AsmPrinter/DwarfDebug.h

   // Whether or not to use AT_ranges for compilation units.
   bool HasCURanges;
 
+  // Whether we emit DWARF linkage name entries (DW_AT_MIPS_linkage_name and 
+  // DW_AT_linkage_name)
+  bool EmitLinkageNames;
+
   // Whether we emitted a function into a section other than the default
   // text.
   bool UsedNonDefaultText;

     LabelsAfterInsn.insert(std::make_pair(MI, nullptr));
   }
 
+  void parseDwarfGenMetadata(const Module *M);
+
 public:
   //===--------------------------------------------------------------------===//
   // Main entry points.

   /// split dwarf proposal support.
   bool useSplitDwarf() const { return HasSplitDwarf; }
 
+  /// \brief Returns whether or not to emit DW_AT_MIPS_linkage_name and 
+  /// DW_linkage_name entries.
+  bool emitLinkageNames() const { return EmitLinkageNames; }
+
   /// Returns the Dwarf Version.
   unsigned getDwarfVersion() const { return DwarfVersion; }
 

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

   else
     HasDwarfPubSections = DwarfPubSections == Enable;
 
+  EmitLinkageNames = true;
+  parseDwarfGenMetadata(M);
+
   unsigned DwarfVersionNumber = Asm->TM.Options.MCOptions.DwarfVersion;
   DwarfVersion = DwarfVersionNumber ? DwarfVersionNumber
                                     : MMI->getModule()->getDwarfVersion();

 
   // If the linkage name is different than the name, go ahead and output
   // that as well into the name table.
-  if (SP.getLinkageName() != "" && SP.getName() != SP.getLinkageName())
+  if (emitLinkageNames() && 
+      SP.getLinkageName() != "" && SP.getName() != SP.getLinkageName())
     addAccelName(SP.getLinkageName(), Die);
 
   // If this is an Objective-C selector name add it to the ObjC accelerator

   AccelTypes.AddName(Name, InfoHolder.getStringPool().getSymbol(*Asm, Name),
                      &Die);
 }
+
+
+// Parse module metadata to deal with options relevant to DWARF generation.
+// The only supported flag is "suppress-linkage-names", anything else is
+// ignored.
+void DwarfDebug::parseDwarfGenMetadata(const Module *M) {
+  Value *Val = M->getModuleFlag("Dwarf Gen Flags");
+  if (Val == nullptr)
+    return;
+  MDNode *DwarfGenFlags = cast<MDNode>(Val);
+  for (unsigned i = 0, e = DwarfGenFlags->getNumOperands(); i != e; ++i) {
+    if (MDString *MDFlag = dyn_cast<MDString>(DwarfGenFlags->getOperand(i))) {
+      if (MDFlag->getString() == "suppress-linkage-names")
+        EmitLinkageNames = false;
+    }
+  }
+}

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

   assert(((LinkageName.empty() || DeclLinkageName.empty()) ||
           LinkageName == DeclLinkageName) &&
          "decl has a linkage name and it is different");
-  if (!LinkageName.empty() && DeclLinkageName.empty())
+  if (DD->emitLinkageNames() && !LinkageName.empty() && DeclLinkageName.empty())
     addString(SPDie, dwarf::DW_AT_MIPS_linkage_name,
               GlobalValue::getRealLinkageName(LinkageName));
 

test/DebugInfo/suppress-linkage-entries.ll

+; REQUIRES: object-emission
+
+; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump - | FileCheck %s
+
+; The test checks whether the DWARF linkage name entries are suppressed 
+; in response to the flag "suppress-linkage-names" which is conveyed
+; by the module flags "Dwarf Gen Flags".
+
+; 
+; The IR was generated with clang -g from the source below with a modified
+; cfe that generates the module flag "Dwarf Gen Flags".
+
+; namespace bar {
+;   int i;
+; }
+; 
+; void foo()
+; {
+; }
+
+; CHECK-NOT: DW_AT_MIPS_linkage_name
+; CHECK-NOT: DW_AT_linkage_name
+
+@_ZN3bar1iE = global i32 0, align 4
+
+; Function Attrs: nounwind uwtable
+define void @_Z3foov() #0 {
+entry:
+  ret void, !dbg !17
+}
+
+attributes #0 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!12, !13, !14}
+!llvm.ident = !{!16}
+
+!0 = metadata !{i32 786449, metadata !1, i32 4, metadata !"clang version 3.6.0 (218613)", i1 false, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !8, metadata !2, metadata !"", i32 1} ; [ DW_TAG_compile_unit ] [/home/user/test/test.cpp] [DW_LANG_C_plus_plus]
+!1 = metadata !{metadata !"test.cpp", metadata !"/home/user/test"}
+!2 = metadata !{}
+!3 = metadata !{metadata !4}
+!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"foo", metadata !"foo", metadata !"_Z3foov", i32 5, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 false, void ()* @_Z3foov, null, null, metadata !2, i32 6} ; [ DW_TAG_subprogram ] [line 5] [def] [scope 6] [foo]
+!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type ] [/home/user/test/test.cpp]
+!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !7, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!7 = metadata !{null}
+!8 = metadata !{metadata !9}
+!9 = metadata !{i32 786484, i32 0, metadata !10, metadata !"i", metadata !"i", metadata !"_ZN3bar1iE", metadata !5, i32 2, metadata !11, i32 0, i32 1, i32* @_ZN3bar1iE, null} ; [ DW_TAG_variable ] [i] [line 2] [def]
+!10 = metadata !{i32 786489, metadata !1, null, metadata !"bar", i32 1} ; [ DW_TAG_namespace ] [bar] [line 1]
+!11 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
+!12 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}
+!13 = metadata !{i32 2, metadata !"Debug Info Version", i32 1}
+!14 = metadata !{i32 6, metadata !"Dwarf Gen Flags", metadata !15}
+!15 = metadata !{metadata !"suppress-linkage-names"}
+!16 = metadata !{metadata !"clang version 3.6.0 (218613)"}
+!17 = metadata !{i32 7, i32 1, metadata !4, null}