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

[Paul Robinson via Phabricator via llvm-commits]

probinson accepted this revision.
probinson added a comment.
This revision is now accepted and ready to land.

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. :-)



================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:128
+    "dwarf-sections-as-references", cl::Hidden,
+    cl::desc("Use sections+|-offset as references rather than labels."),
+    cl::values(clEnumVal(Default, "Default for platform"),
----------------
I'd prefer "section+offset" as I doubt you would ever have a reference to something before the start of a section!


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:320
 
+  // Use sections as references in DWARF2 or less.
+  UseSectionsAsReferences =
----------------
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.


================
Comment at: test/DebugInfo/X86/sections_as_references.ll:6
+; CHECK-NEXT: .file 2 "/dir/tu1.cpp"
+; CHECK-NEXT: .file 3 "/dir/tu2.cpp"
+
----------------
Do you really need to check for .file directives?


================
Comment at: test/DebugInfo/X86/sections_as_references.ll:26
+; CHECK-NOT:  .L
+; CHECK:      .quad .debug_info+57 # DW_AT_type
+; CHECK-NOT:  .L
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D43627