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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev marked an inline comment as done.
ABataev added inline comments.


================
Comment at: include/llvm/MC/MCAsmInfo.h:586
   bool useParensForSymbolVariant() const { return UseParensForSymbolVariant; }
+  bool supportsExtendedDwarfLocDirective() const {
+    return SupportsExtendedDwarfLocDirective;
----------------
echristo wrote:
> Let's separate this out into a separate patch as well?
Ok


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:333
                                     : MMI->getModule()->getDwarfVersion();
-  // Use dwarf 4 by default if nothing is requested.
-  DwarfVersion = DwarfVersion ? DwarfVersion : dwarf::DWARF_VERSION;
+  // Use dwarf 4 by default if nothing is requested. For NVPTX, use dwarf 2.
+  DwarfVersion =
----------------
echristo wrote:
> Do you still need to use dwarf2 here?
Yes, we need to force dwarf2 for NVPTX, this is what we agreed on with Paul Robinson reviewing clang part of the patch


================
Comment at: lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.h:33
+  /// same level as kernel and device function declarations.
+  /// LLVM emits .file directives immediately the location debug info is
+  /// emitted, i.e. they may be emitted inside functions. We gather all these
----------------
echristo wrote:
> So, this functionality is used for changing files within the line table. I think what's actually going on here is that nvptx, for some reason, only supports a single file directive.
> 
> Basically you don't support the idea of:
> 
> int foo (void) {
>   // some code
>   #include "somefile.def"
>   // some more code
> }
> 
> having a file switch in the middle of a function. That's arguably a bug, but I think you don't want to collect them and emit them outside the functions because then it would be more actively wrong.
No, it means that you may have the reference to the `somefile.def`, but the table of files should be emitted outside of the function, in the outer context


================
Comment at: lib/Target/NVPTX/NVPTXAsmPrinter.cpp:477
+  // Emit open brace for function body.
   OutStreamer->EmitRawText(StringRef("{\n"));
   setAndEmitFunctionVirtualRegisters(*MF);
----------------
echristo wrote:
> tra wrote:
> > Please add a comment here that the closing brace is emitted at the end of NVPTXAsmPrinter::runOnMacheineFunction(). 
> Seems odd that we're pulling the functionality across multiple functions that don't really work in pairs. 
Yes, maybe it is worth it to add some option that allows enclosing of the dwarf sections into braces on the emission?


Repository:
  rL LLVM

https://reviews.llvm.org/D41827