<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Mar 10, 2014 at 4:37 PM, Eli Bendersky <span dir="ltr"><<a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Mon, Mar 10, 2014 at 1:20 PM, Justin Holewinski <span dir="ltr"><<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Mon, Mar 10, 2014 at 4:15 PM, Eli Bendersky <span dir="ltr"><<a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div>On Mon, Mar 10, 2014 at 12:26 PM, Justin Holewinski <span dir="ltr"><<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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. :)<br>
</div></blockquote><div><br></div></div><div>Oops, sorry about the reverse patch. This is now committed in r203483.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div><br>The patch LGTM. I'm still concerned about collisions, but I agree that this is better than what we currently have.<br><br></div><div>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.<br>
<br>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.<br>
</div></div></blockquote><div><br></div></div><div>I opened PR19099 to keep track of this. A pre-codegen IR pass makes sense to me. However, what about external symbols? In theory, they are useless if ptxas can't assemble them anyway.</div>
</div></div></div></blockquote><div><br></div></div></div><div>Right, if a symbol is internal, we can mangle/unique it. But if its externally-defined or externally-visible, we can't do anything to it. It's not perfect, but its likely the best we can do given the current constraints in the PTX assembler.</div>
</div></div></div></blockquote><div><br></div></div></div><div>Just to examine the possibilities in a bit more detail: since the problematic characters in question are "." and "@", and these can't be part of a valid CUDA program, then as you pointed out, they can only sneak in via LLVM IR. This means NVPTX is involved. So there are two options:</div>
<div><br></div><div>1. Don't mangle external symbols that contain forbidden chars.</div><div>2. Do mangle all symbols -- external and internal.</div><div><br></div><div>(1) means that a PTX file simply won't compile, so it will be unusable completely. (2) means that the PTX file will compile, but if the external symbol is explicitly referenced, it won't be found, because its name has changed. This can still be worked around by manually mangling the name given to the driver API, right?</div>
<div><br></div><div>Yes, this is ugly and horrible and all, but it's not like we've never seen manually-mangled C++ names appear in C interfaces (to access stuff where "extern C" was emitted for some reason). The point is, this can be made to work *somehow*, as opposed to the alternative -- (1) which means things won't work at all.</div>
<div><br></div><div>What am I missing?</div></div></div></div></blockquote><div><br></div><div>CUDA C is only one possible producer of LLVM IR. Other languages may not follow the C++ rules for identifiers. If we cannot produce a symbol name in PTX for an externally-visible symbol, I would prefer to error out instead of generate PTX that exposes symbols names different from what the input IR specified. So we could error out for external symbols (with an appropriate message about the form external symbols must take) and mangle internal symbols.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Naturally, this all can go away if ptxas grows an ability to do some rudimentary quoting :)</div>
<span class="HOEnZb"><font color="#888888"><div><br></div><div>Eli</div><div><br></div><div><br></div>
<div>
<br></div><div><br></div><div><br></div><div><br></div><div><br></div><div> </div></font></span></div></div></div>
</blockquote></div><br><br clear="all"><br>-- <br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div>
</div></div>