PATCH: Make sure NVPTX doesn't emit symbols that aren't valid for ptxas
justin.holewinski at gmail.com
Mon Mar 10 12:26:10 PDT 2014
I had to ponder the patch a bit before I realized it was a reverse patch
and you were adding the getSymbolName function. I was trying to figure out
why we had an issue in the first place if this function already existed,
any why you were reverting it to go back to the Mangler interface. :)
The patch LGTM. I'm still concerned about collisions, but I agree that
this is better than what we currently have.
An alternative could be to implement a pre-codegen IR pass that renames
globals to match what is representable in PTX. This would not incur any
additional memory cost (other than potentially increasing the size of some
symbol names), and provide an easy way to catch (and unique) collisions.
Though for correctness, the variables should only be renamed if they are
internal. In theory, this is currently broken because you could create a
symbol ".foo" at the IR level and try to get its address using the CUDA
driver API, but fail since the symbol name would really be _$_foo in the
binary. Perhaps we could loudly fail if an externally visible symbol would
need mangling/uniquing, and just mangle/unique the symbol if its internal.
That's more long term, though. Feel free to commit this patch.
On Mon, Mar 10, 2014 at 2:52 PM, Eli Bendersky <eliben at google.com> wrote:
> Hi Justin,
> Please find attached an initial patch for what we've discussed earlier. It
> currently simply replaces the invalid symbols (only '.' and '@' may come
> from MCSymbol) with a different sequence. Potentially, this can create
> collisions, but since ptxas doesn't have general quoting, this is hard to
> work around. In any case, it's strictly better than the current situation
> which creates invalid assembly.
> A more thorough approach could be to keep a map remembering symbol names
> and failing loudly in case of collisions. This has a memory usage cost.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits