[DEBUGINFO] Add flag for DWARF2 or less to use sections as references.

[ABataev]

Summary

Some targets does not support labels inside debug sections, but support
references in form section +|- offset. Patch adds initial support
for this. Also, this patch disables emission of all additional debug

sections that may have labels inside of it (like pub sections and
string tables).

[ABataev]

Ping

[probinson]

LGTM with minor changes.
I didn't try to verify that you found all the places where this check would be needed; I trust that you will trip over them if you missed any. :-)

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

128 ↗

(On Diff #135440)

I'd prefer "section+offset" as I doubt you would ever have a reference to something before the start of a section!

320 ↗

(On Diff #135440)

s/DWARF2 or less/DWARF v2/
i.e. use the preferred version spelling, and there is no "less."
The <= 2 below could also be simply == 2 but that's up to you.

test/DebugInfo/X86/sections_as_references.ll

6 ↗

(On Diff #135440)

Do you really need to check for .file directives?

26 ↗

(On Diff #135440)

I think the exact offset is not important? Just that there is one. If that's the case, then using a regex for the offset will be less sensitive to irrelevant changes in emitting debug info.

[JDevlieghere]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

274 ↗

(On Diff #135440)

The LLVM coding standard says not to put braces around single statement blocks.

[ABataev]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

274 ↗

(On Diff #135440)

Only if there is no else branch with multiple line substatement. In this case, both substatements must be enclosed in braces.

[JDevlieghere]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

274 ↗

(On Diff #135440)

But that's not the case here, is it?

[ABataev]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

274 ↗

(On Diff #135440)

Why not? The else substatement is 2-line.

[ABataev]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

128 ↗

(On Diff #135440)

Ok, will do

test/DebugInfo/X86/sections_as_references.ll

6 ↗

(On Diff #135440)

No, will remove these checks

[echristo]

Using sections as references is, in general, a good idea when we can so I'd rather not tie that to whether or not we emit certain sections. If you don't mind reverting that aspect of the change I'd appreciate it and we can get that in via some other preference or a strict requirement via the asm printer as we're planning on doing for the rest of the nvptx section emission.

[ABataev]

In D43627#1022609, @echristo wrote:

Using sections as references is, in general, a good idea when we can so I'd rather not tie that to whether or not we emit certain sections. If you don't mind reverting that aspect of the change I'd appreciate it and we can get that in via some other preference or a strict requirement via the asm printer as we're planning on doing for the rest of the nvptx section emission.

These sections have labels inside of them + they are not supported by PTX format. Ok, I will revert the patch.

[JDevlieghere]

I really hate to go on about this, but the truth is that I've been told this so many times that it instantly caught my eye. 😀

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

274 ↗

(On Diff #135440)

It's still a single statement, only it that happens to be split across two lines, which doesn't affect whether we need braces.

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1580 ↗

(On Diff #135440)

Same here

1584 ↗

(On Diff #135440)

And here

[ABataev]

In D43627#1022846, @JDevlieghere wrote:

I really hate to go on about this, but the truth is that I've been told this so many times that it instantly caught my eye. 😀

1. Several years ago I got a lot of reviews where reviewers insisted(!) to enclose multiline (not multistatement!) single-statement substatements into braces. I did for the last 6 years and had no problems with it. This is the first time I see this. Di I miss something, the rules changed already?
2. There is a patch D26943, that was accepted but not committed to enclosing multiline (not multistatement) substatements in if and for statements. It is clear that multistatement substatements must be enclosed and this kind of rule is not required at all.

I'm not against this kind of change but seems to me this is not strictly standardized.

[dblaikie]

In fact none of this is in the style guide itself, it's just lore and the
usual argument for consistency with surrounding code.

There are many examples of most points on this spectrum - the generally
agreed upon bit is "single line blocks (without connected/related blocks -
like else, etc) shouldn't have braces". Whether you brace multiline, single
statement blocks, or brace all related blocks if one of them is braced, is
much more variable and I generally leave up to the author unless there's
some really clear consistency argument to be made.

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (CURanges.empty() || !SameAsPrevCU ||
     return;
   }
 
   CURanges.back().setEnd(Range.getEnd());
 }
 
 void DwarfCompileUnit::initStmtList() {
   // Define start line table label for each Compile Unit.
-  MCSymbol *LineTableStartSym =
+  MCSymbol *LineTableStartSym;
+  const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
+  if (DD->useSectionsAsReferences()) {
+    LineTableStartSym = TLOF.getDwarfLineSection()->getBeginSymbol();
+  } else {
+    LineTableStartSym =
         Asm->OutStreamer->getDwarfLineTableSymbol(getUniqueID());
+  }
 
   // DW_AT_stmt_list is a offset of line number information for this
   // compile unit in debug_line section. For split dwarf this is
   // left in the skeleton CU and so not included.
   // The line table entries are not always emitted in assembly, so it
   // is not okay to use line_table_start here.
-  const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
   StmtListValue =
       addSectionLabel(getUnitDie(), dwarf::DW_AT_stmt_list, LineTableStartSym,
                       TLOF.getDwarfLineSection()->getBeginSymbol());
 }
 
 void DwarfCompileUnit::applyStmtList(DIE &D) {
   D.addValue(DIEValueAllocator, *StmtListValue);
 }
addSectionLabel(ScopeDIE, dwarf::DW_AT_ranges, List.getSym(),
                     RangeSectionSym);
 
   // Add the range list to the set of ranges to be emitted.
   (Skeleton ? Skeleton : this)->CURangeLists.push_back(std::move(List));
 }
 
 void DwarfCompileUnit::attachRangesOrLowHighPC(
     DIE &Die, SmallVector<RangeSpan, 2> Ranges) {
-  if (Ranges.size() == 1) {
-    const auto &single = Ranges.front();
-    attachLowHighPC(Die, single.getStart(), single.getEnd());
+  if (Ranges.size() == 1 || DD->useSectionsAsReferences()) {
+    const auto &front = Ranges.front();
+    const auto &back = Ranges.back();
+    attachLowHighPC(Die, front.getStart(), back.getEnd());
   } else
     addScopeRangeList(Die, std::move(Ranges));
 }
 
 void DwarfCompileUnit::attachRangesOrLowHighPC(
     DIE &Die, const SmallVectorImpl<InsnRange> &Ranges) {
   SmallVector<RangeSpan, 2> List;
   List.reserve(Ranges.size());
void DwarfCompileUnit::createAbstractVariable(const DILocalVariable *Var,
   assert(Scope && Scope->isAbstractScope());
   auto AbsDbgVariable = llvm::make_unique<DbgVariable>(Var, /* IA */ nullptr);
   DU->addScopeVariable(Scope, AbsDbgVariable.get());
   getAbstractVariables()[Var] = std::move(AbsDbgVariable);
 }
 
 void DwarfCompileUnit::emitHeader(bool UseOffsets) {
   // Don't bother labeling the .dwo unit, as its offset isn't used.
-  if (!Skeleton) {
+  if (!Skeleton && !DD->useSectionsAsReferences()) {
     LabelBegin = Asm->createTempSymbol("cu_begin");
     Asm->OutStreamer->EmitLabel(LabelBegin);
   }
 
   dwarf::UnitType UT = Skeleton ? dwarf::DW_UT_split_compile
                                 : DD->useSplitDwarf() ? dwarf::DW_UT_skeleton
                                                       : dwarf::DW_UT_compile;
   DwarfUnit::emitCommonHeader(UseOffsets, UT);
 }
 
 bool DwarfCompileUnit::hasDwarfPubSections() const {
   // Opting in to GNU Pubnames/types overrides the default to ensure these are
   // generated for things like Gold's gdb_index generation.
   if (CUNode->getGnuPubnames())
     return true;
 
-  return DD->tuneForGDB() && !includeMinimalInlineScopes();
+  return DD->tuneForGDB() && !includeMinimalInlineScopes() &&
+         !DD->useSectionsAsReferences();
 }
 
 /// addGlobalName - Add a new global name to the compile unit.
 void DwarfCompileUnit::addGlobalName(StringRef Name, const DIE &Die,
                                      const DIScope *Context) {
   if (!hasDwarfPubSections())
     return;
   std::string FullName = getParentContextString(Context) + Name.str();

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h

class DwarfDebug : public DebugHandlerBase {
   bool UseDWARF2Bitfields;
 
   /// Whether to emit all linkage names, or just abstract subprograms.
   bool UseAllLinkageNames;
 
   /// Use inlined strings.
   bool UseInlineStrings = false;
 
+  /// True if the sections itself must be used as references and don't create
+  /// temp symbols inside DWARF sections.
+  bool UseSectionsAsReferences = false;
+
   /// DWARF5 Experimental Options
   /// @{
   bool HasDwarfAccelTables;
   bool HasAppleExtensionAttributes;
   bool HasSplitDwarf;
 
   /// Whether to generate the DWARF v5 string offsets table.
   /// It consists of a series of contributions, each preceded by a header.
public:
 
   /// Returns whether to use the DWARF2 format for bitfields instyead of the
   /// DWARF4 format.
   bool useDWARF2Bitfields() const { return UseDWARF2Bitfields; }
 
   /// Returns whether to use inline strings.
   bool useInlineStrings() const { return UseInlineStrings; }
 
+  /// Returns whether to use sections as labels rather than temp symbols.
+  bool useSectionsAsReferences() const {
+    return UseSectionsAsReferences;
+  }
+
   // Experimental DWARF5 features.
 
   /// Returns whether or not to emit tables that dwarf consumers can
   /// use to accelerate lookup.
   bool useDwarfAccelTables() const { return HasDwarfAccelTables; }
 
   bool useAppleExtensionAttributes() const {
     return HasAppleExtensionAttributes;

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 static cl::opt<DefaultOnOff>
 DwarfInlinedStrings("dwarf-inlined-strings", cl::Hidden,
                  cl::desc("Use inlined strings rather than string section."),
                  cl::values(clEnumVal(Default, "Default for platform"),
                             clEnumVal(Enable, "Enabled"),
                             clEnumVal(Disable, "Disabled")),
                  cl::init(Default));
 
+static cl::opt<DefaultOnOff> DwarfSectionsAsReferences(
+    "dwarf-sections-as-references", cl::Hidden,
+    cl::desc("Use sections+offset as references rather than labels."),
+    cl::values(clEnumVal(Default, "Default for platform"),
+               clEnumVal(Enable, "Enabled"), clEnumVal(Disable, "Disabled")),
+    cl::init(Default));
+
 enum LinkageNameOption {
   DefaultLinkageNames,
   AllLinkageNames,
   AbstractLinkageNames
 };
 
 static cl::opt<LinkageNameOption>
     DwarfLinkageNames("dwarf-linkage-names", cl::Hidden,
else
     UseAllLinkageNames = DwarfLinkageNames == AllLinkageNames;
 
   unsigned DwarfVersionNumber = Asm->TM.Options.MCOptions.DwarfVersion;
   unsigned DwarfVersion = DwarfVersionNumber ? DwarfVersionNumber
                                     : MMI->getModule()->getDwarfVersion();
   // Use dwarf 4 by default if nothing is requested.
   DwarfVersion = DwarfVersion ? DwarfVersion : dwarf::DWARF_VERSION;
 
+  // Use sections as references in DWARF v2.
+  UseSectionsAsReferences =
+      DwarfVersion == 2 && DwarfSectionsAsReferences == Enable;
+
   // Work around a GDB bug. GDB doesn't support the standard opcode;
   // SCE doesn't support GNU's; LLDB prefers the standard opcode, which
   // is defined as of DWARF 3.
   // See GDB bug 11616 - DW_OP_form_tls_address is unimplemented
   // https://sourceware.org/bugzilla/show_bug.cgi?id=11616
   UseGNUTLSOpcode = tuneForGDB() || DwarfVersion < 3;
 
   // GDB does not fully support the DWARF 4 representation for bitfields.
for (const auto &P : CUMap) {
     // If we have code split among multiple sections or non-contiguous
     // ranges of code then emit a DW_AT_ranges attribute on the unit that will
     // remain in the .o file, otherwise add a DW_AT_low_pc.
     // FIXME: We should use ranges allow reordering of code ala
     // .subsections_via_symbols in mach-o. This would mean turning on
     // ranges for all subprogram DIEs for mach-o.
     DwarfCompileUnit &U = SkCU ? *SkCU : TheCU;
     if (unsigned NumRanges = TheCU.getRanges().size()) {
-      if (NumRanges > 1)
+      if (NumRanges > 1 && !useSectionsAsReferences())
         // A DW_AT_low_pc attribute may also be specified in combination with
         // DW_AT_ranges to specify the default base address for use in
         // location lists (see Section 2.6.2) and range lists (see Section
         // 2.17.3).
         U.addUInt(U.getUnitDie(), dwarf::DW_AT_low_pc, dwarf::DW_FORM_addr, 0);
       else
         U.setBaseAddress(TheCU.getRanges().front().getStart());
       U.attachRangesOrLowHighPC(U.getUnitDie(), TheCU.takeRanges());
void DwarfDebug::emitDebugPubSection(bool GnuStyle, StringRef Name,
   Asm->EmitLabelDifference(EndLabel, BeginLabel, 4);
 
   Asm->OutStreamer->EmitLabel(BeginLabel);
 
   Asm->OutStreamer->AddComment("DWARF Version");
   Asm->EmitInt16(dwarf::DW_PUBNAMES_VERSION);
 
   Asm->OutStreamer->AddComment("Offset of Compilation Unit Info");
+  if (useSectionsAsReferences()) {
+    Asm->EmitLabelPlusOffset(TheU->getSection()->getBeginSymbol(),
+                             TheU->getDebugSectionOffset(),
+                             Asm->MAI->getCodePointerSize());
+  } else {
     Asm->emitDwarfSymbolReference(TheU->getLabelBegin());
+  }
 
   Asm->OutStreamer->AddComment("Compilation Unit Length");
   Asm->EmitInt32(TheU->getLength());
 
   // Emit the pubnames for this compilation unit.
   for (const auto &GI : Globals) {
     const char *Name = GI.getKeyData();
     const DIE *Entity = GI.second;
for (DwarfCompileUnit *CU : CUs) {
     ContentSize += (List.size() + 1) * TupleSize;
 
     // For each compile unit, write the list of spans it covers.
     Asm->OutStreamer->AddComment("Length of ARange Set");
     Asm->EmitInt32(ContentSize);
     Asm->OutStreamer->AddComment("DWARF Arange version number");
     Asm->EmitInt16(dwarf::DW_ARANGES_VERSION);
     Asm->OutStreamer->AddComment("Offset Into Debug Info Section");
+    if (useSectionsAsReferences()) {
+      Asm->EmitLabelPlusOffset(CU->getSection()->getBeginSymbol(),
+                               CU->getDebugSectionOffset(),
+                               Asm->MAI->getCodePointerSize());
+    } else {
       Asm->emitDwarfSymbolReference(CU->getLabelBegin());
+    }
     Asm->OutStreamer->AddComment("Address Size (in bytes)");
     Asm->EmitInt8(PtrSize);
     Asm->OutStreamer->AddComment("Segment Size (in bytes)");
     Asm->EmitInt8(0);
 
     Asm->OutStreamer->emitFill(Padding, 0xff);
 
     for (const ArangeSpan &Span : List) {

llvm/trunk/test/DebugInfo/X86/sections_as_references.ll

+; RUN: llc -filetype=asm -O0 -mtriple=x86_64-linux-gnu < %s -dwarf-sections-as-references=Enable -dwarf-inlined-strings=Enable -dwarf-version 2 | FileCheck %s
+
+; CHECK:      .file
+
+; CHECK-NOT:  .L
+
+; CHECK:      .section .debug_abbrev
+; CHECK-NOT:  DW_FORM_str{{p|x}}
+; CHECK-NOT: .L
+
+; CHECK:      .section .debug_info
+; CHECK-NOT:  .L
+; CHECK:      .short 2             # DWARF version number
+; CHECK-NOT:  .L
+; CHECK:      .long .debug_abbrev  # Offset Into Abbrev. Section
+; CHECK-NOT:  .L
+; CHECK:      .long .debug_line    # DW_AT_stmt_list
+; CHECK-NOT:  .L
+; CHECK:      .long .debug_abbrev  # Offset Into Abbrev. Section
+; CHECK-NOT:  .L
+; CHECK:      .long .debug_line    # DW_AT_stmt_list
+; CHECK-NOT:  .L
+; CHECK:      .quad .debug_info+{{[0-9]+}} # DW_AT_type
+; CHECK-NOT:  .L
+; CHECK:      .byte 0              # End Of Children Mark
+; CHECK-NOT:  .L
+
+source_filename = "test/DebugInfo/X86/sections_as_references.ll"
+
+%struct.foo = type { i8 }
+
+@f = global %struct.foo zeroinitializer, align 1, !dbg !0
+@g = global %struct.foo zeroinitializer, align 1, !dbg !6
+
+!llvm.dbg.cu = !{!9, !12}
+!llvm.module.flags = !{!14, !15}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "f", scope: null, file: !2, line: 2, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "tu1.cpp", directory: "/dir")
+!3 = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", file: !4, line: 1, size: 8, align: 8, elements: !5, identifier: "_ZTS3foo")
+!4 = !DIFile(filename: "./hdr.h", directory: "/dir")
+!5 = !{}
+!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+!7 = !DIGlobalVariable(name: "g", scope: null, file: !8, line: 2, type: !3, isLocal: false, isDefinition: true)
+!8 = !DIFile(filename: "tu2.cpp", directory: "/dir")
+!9 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !2, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !5, retainedTypes: !10, globals: !11, imports: !5)
+!10 = !{!3}
+!11 = !{!0}
+!12 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !8, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !5, retainedTypes: !10, globals: !13, imports: !5)
+!13 = !{!6}
+!14 = !{i32 2, !"Dwarf Version", i32 2}
+!15 = !{i32 1, !"Debug Info Version", i32 3}
+