A global variable may have the same name as a label, and ptxas does not accept it.
Prefix labels with $L__ to fix this.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I know nearly nothing about NVPTX.
GNU assembler calls this property "local label prefix" (https://sourceware.org/binutils/docs/as/Symbol-Names.html#Symbol-Names), which is typically .L on ELF.
This is usually specified by the assembler manual or psABI for the target. For example. AArch64 psABI has a note (can be seen as nonnormative) that ARMCC uses L.
Does NVPTX has such a specification? Also, if you grep PrivateGlobalPrefix = , having such a long PrivateGlobalPrefix is an outlier.
LGTM.
Official spec is here: https://docs.nvidia.com/cuda/parallel-thread-execution/index.html
Unfortunately, it does not say much about labels and whether they have a concept of a local labal.
Also, if you grep PrivateGlobalPrefix = , having such a long PrivateGlobalPrefix is an outlier.
PTX is a rather quirky assembly, so being an outlier is business as usual. :-/
For what it's worth, $L__ is what NVCC uses since CUDA-11.3 : https://godbolt.org/z/3q4be9PrG.
I guess $ alone or $L may be sufficient to avoid name conflicts, but $L__ is fine, too, IMO.
llvm/test/CodeGen/NVPTX/label-var-prefix.ll | ||
---|---|---|
17 ↗ | (On Diff #408274) | You should also match the trailing : otherwise the check could match one of the the label uses. |
Thanks. If there is any channel communicating with them, it'd be nice to have this documented somewhere.
Also, if you grep PrivateGlobalPrefix = , having such a long PrivateGlobalPrefix is an outlier.
PTX is a rather quirky assembly, so being an outlier is business as usual. :-/
For what it's worth, $L__ is what NVCC uses since CUDA-11.3 : https://godbolt.org/z/3q4be9PrG.
I guess $ alone or $L may be sufficient to avoid name conflicts, but $L__ is fine, too, IMO.
Cool. If NVCC uses $L__, this is definitely safe.
@slydiman please update the message to mention that the choice of $L__ matches NVCC (not LLVM local invention).
Looks like it broke CUDA:
https://lab.llvm.org/buildbot/#/builders/46/builds/23372/steps/3/logs/stdio
ptxas /tmp/complex-78abf1/complex-sm_75.s, line 250; fatal : Parsing error near '$L__BB6_2': syntax error
Thank you for reverting the patch.
@slydiman please verify that clang-generated PTX with the patch does get accepted by ptxas. I'm not quite sure what went wrong here and we can't test it in clang/llvm in-tree tests as they don't have access to CUDA SDK.
Can you elaborate on what exactly is failing with older versions? AFAICT, $... is accepted by ptxas going back as far as CUDA-9.0: https://godbolt.org/z/G5n7E9xYz
On a side note, it's apparently possible to have a global or function to start with $: https://godbolt.org/z/zW41xPnPd, https://godbolt.org/z/srjdh3a46
So, prefixing with $ does not solve the problem completely.
We may be better off using % or just __ as the prefix. While it's possible to have a conflict with the latter one, at least the end users are not supposed to use it.
I believe the problem is solved here https://reviews.llvm.org/D123702
I will check everything and resubmit this patch.
Indeed it's still possible to break compilation for both clang and nvcc: https://godbolt.org/z/9h649eMMn
I guess that $__ prefix was chosen because it matches nvcc.
We may be better off using % or just __ as the prefix. While it's possible to have a conflict with the latter one, at least the end users are not supposed to use it.
Using % instead of $ sounds like a good idea - it is not a valid identifier in C, and it seems to be supported by ptxas.