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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev marked 2 inline comments as done.
ABataev added a comment.

In https://reviews.llvm.org/D41827#977422, @tra wrote:

> Looks OK to me. That said,  I have little clue about DWARF, so I'll defer to echristo@ as it's his domain.
>
> In general, it would be good if we could generate debug info in a somewhat more compact way (more than one byte per .b8 directive, for example) -- debug info has tendency to grow and we do carry PTX around in compiled binaries.


Ok, will try to reduce the size in future patches.



================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.h:263
+  /// temp symbols inside DWARF sections.
+  bool UseSectionsLabelsAsReferences = false;
+
----------------
tra wrote:
> Nit: Should it be section (singular) labels (plural) here? E.g. we have license plates and book covers. 
> 
> Perhaps it should be just `UseSectionsAsReferences`.
Ok


================
Comment at: lib/Target/NVPTX/NVPTXAsmPrinter.h:350
+    bool Result = AsmPrinter::runOnMachineFunction(F);
+    OutStreamer->EmitRawText(StringRef("}\n"));
+    return Result;
----------------
tra wrote:
> Do I understand it correctly that printout of the `}` was moved here out of `EmitFunctionBodyEnd()` so that we can emit additional debug labels after the last basic block?
> 
> It would be good to add few comments describing why we emit (or not) braces in particular places.
Yes, correct.
Ok, will add


https://reviews.llvm.org/D41827