Improve debug info for kernel arguments#242
Conversation
| """ | ||
| assert not noalias | ||
| arginfo = self._get_arg_packer(fe_argtypes) | ||
| # Do not prefix "arg." on argument name, so that nvvm compiler |
There was a problem hiding this comment.
I think that it's OK not to prefix arg. onto the name - as far as I can tell it's a convention that was added that has not become load-bearing in any way - I don't think a duplicate name will appear inside a function body.
|
|
||
| # Create metadata type for return value | ||
| if len(llfunc.args) > 0: | ||
| lltype = llfunc.args[0].type |
There was a problem hiding this comment.
Does omitting the replacement of . with $ compared to the original implementation not cause issues for GDB? I note the comment in the original version:
name = llarg.name.replace('.', '$') # for gdb to work correctlyI thought that a dot in a name might be problematic for it? (c.f. the same syntax for traversing the members of a struct or union)
There was a problem hiding this comment.
That . versioned name used to be written to the metadata node as the value of 'name' of DILocalVariable. But currently the . versioned variable nodes are merged to the unversioned / user symbol named metadata node. Therefore, they are no longer to be present in DWARF entry.
And when represented as member of a struct or union, their names in the metadata are also unversioned names with corresponding types, so there won't be issues omitting this original replacement.
In short, neither . nor $ will show up in debugger.
gmarkall
left a comment
gmarkall
left a comment
There was a problem hiding this comment.
I think this looks good in general - I do have one question on the diff, and I'd like to understand if / why it's OK.
gmarkall
added
4 - Waiting on author
gmarkall
added
5 - Ready to merge
- Allow External Code to Use Cooperative Group (#240) - Improve debug info for kernel arguments (#242) - Allow Numba NVRTC Binding Search Additional Paths (#254) - Add Bfloat16 High Level API, Documentation (#245) - add a test to use bf16 bindings inside device functions (#244) - Change CI to only be manually triggered to save on CI runs (#252) - Simplify the CI build and test matrix (#249)
2 participants
In order to leverage more accurate debug info tracking from the nvvm compiler, kernel argument name need to be exact matching the name from the metadata, which is using source level user symbol.
(1) Change the CUDACallConv on CUDA target to not prefix "arg." on argument name;
(2) Call CUDA DIBuilder to build up customized DISubroutineType which is different than the parent method;
(3) Add a debug info unit test to guard this change.
This fixes nvbug#5027369.