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

Fri Jan 19 07:02:46 PST 2018

ABataev added a comment.

Hi Eric, thanks for the review.

In https://reviews.llvm.org/D41827#980942, @echristo wrote:

> In general, I think this patch needs a lot more description and possibly breaking up. For now a few questions:

Ok, I'll add some description. As to breaking it up, it will be very hard to do. This patch adds commented out debug info, that allows successful compiling of resulting PTX file. If we will remove at least some small change from this patch, it will produce incorrect PTX file. So, I rather doubt that it is possible to break this patch into several small patches.

> a) nvptx doesn't allow a debug_str section?

Here is an excerpt from `CUDA Toolkit documentation` (http://docs.nvidia.com/cuda/ptx-writers-guide-to-interoperability/index.html#debug-information): `The PTX producer is responsible for emitting binary DWARF into the PTX file, using the .section and .b8-.b16-.b32-and-.b64 directives in PTX. This should contain the .debug_info and .debug_abbrev sections, and possibly optional sections .debug_pubnames and .debug_aranges.` So, yes, .`debug_str` is not allowed.

> b) relocations in nvptx should be against the section rather than the label? (To be fair, they can be this way on elf platforms as well which would be an improvement)

According to `PTX ISA, 11.5.2. Debugging Directives: .section`(http://docs.nvidia.com/cuda/parallel-thread-execution/index.html#debugging-directives) labels are not allowed inside the debugging directives, so we cannot use labels.

> c) There are a bunch of directives for nvptx that you want to use for emission?

Just `.debug_info` and `.debug_abbrevs` sections, all other sections should be empty, if emitted.

> d) It looks like you've got this turned off by default by commenting out everything? I'd rather you just conditionalize it and have it all work and be testable before turning on.

It is just an initial patch. It adds some default debug info generation, but this info is still incorrect. This is why I commented it out in the output PTX. After this patch I need to commit several patches to LLVM and couple patches to clang to be able to produce correct debug info, that is compilable and does not break cuda-gdb and ptxas.
We still can test it, using LLVM lit tests, but it definitely won't work with the cuda-gdb and ptxas.

> I think I'd probably start splitting up the NVPTX asm printer patch into separate and testable patches, perhaps by rewriting the existing line table handling and going from there?

I think the only thing I can do separately is to remove an existing line table handling completely. If this is ok, I can prepare the patch for it.

> I've also made some inline comments of things that were confusing or otherwise not sure about if you also want to tackle those.
> 
> Thanks!
> 
> -eric

Thanks

Repository:
  rL LLVM

https://reviews.llvm.org/D41827