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

[Eric Christopher via Phabricator via llvm-commits]

echristo added inline comments.


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


================
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 =
----------------
Do you still need to use dwarf2 here?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:274
+    DebuggerTuning = DebuggerKind::GDB;
+  else if (Asm->TM.Options.DebuggerTuning != DebuggerKind::Default)
     DebuggerTuning = Asm->TM.Options.DebuggerTuning;
----------------
ABataev wrote:
> probinson wrote:
> > Unconditionally ignoring a command-line option?  Are there tests that fail if you don't do this?
> I'm not aware of any, did it just in case if there are such cases.
Yeah, I don't think this should be here.


================
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
----------------
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.


================
Comment at: lib/Target/NVPTX/NVPTXAsmPrinter.cpp:477
+  // Emit open brace for function body.
   OutStreamer->EmitRawText(StringRef("{\n"));
   setAndEmitFunctionVirtualRegisters(*MF);
----------------
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. 


Repository:
  rL LLVM

https://reviews.llvm.org/D41827