[PATCH] D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:416
     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();
----------------
echristo wrote:
> I don't believe that this is a necessary or sufficient check here. What's the point of it?
PTX format does not allow use of `.debug_ranges` section, we should not emit it.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:760
+    const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
+    if (DD->useSectionsAsReferences()) {
+      LabelBegin = TLOF.getDwarfInfoSection()->getBeginSymbol();
----------------
echristo wrote:
> So there's a difference between a begin symbol for a section and the section itself. You're doing the former and not the latter it looks like? And also I think you're breaking the possibility of multiple cus in a single module.
I checked it, multiple CUs are handled correctly. But the reference to CU is generated as `.b64 .debug_info+offset`, which is supported by PTX. Don't forget, that PTX format is very limited in support of DWARF sections and we will genarte only `.debug_abbrevs` and `.debug_info` sections.


================
Comment at: lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.cpp:36
+                                        raw_ostream &OS) {
+  // FIXME: remove comment once debug info is properly supported.
+  assert(!SubSection && "SubSection is not null!");
----------------
echristo wrote:
> This isn't really a condition? :)
I mean remove comments from the emitted DWARF sections once they are generated correctly.


================
Comment at: lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.cpp:43
+    OS << "//\t}\n";
+  if (Section != FI->getTextSection() && Section != FI->getDataSection() &&
+      Section != FI->getBSSSection()) {
----------------
echristo wrote:
> I don't understand what's going on here?
The body of the DWARF sections in PTX file must be enclosed into braces, like this:
```
.section .debug_info
{
...
}
```
This code checks that current section is one of the DWARF sections and encloses the content of these sections into braces.


================
Comment at: lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.h:27
+
+  /// Record DWARF file directives for later output.
+  void emitDwarfFileDirective(StringRef Directive) override;
----------------
echristo wrote:
> A description of how this works would be good.
Ok, will add


================
Comment at: lib/Target/NVPTX/NVPTXAsmPrinter.cpp:875
 
-  if (MAI->doesSupportDebugInformation())
-    O << ", debug";
+  // FIXME: remove comment once debug info is properly supported.
+  if (MMI && MMI->hasDebugInfo())
----------------
echristo wrote:
> I'm not sure what this fixme means?
I mean, that the comment symbols `//` from the emitted `//, debug` must be removed once the debug info is properly generated. 


Repository:
  rL LLVM

https://reviews.llvm.org/D41827