[PATCH] D137154: Adding nvvm_reflect clang builtin

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: clang/include/clang/Basic/BuiltinsNVPTX.def:827
 
+BUILTIN(__nvvm_reflect, "icC*", "nc")
+
----------------
hdelan wrote:
> tra wrote:
> > Do we actually need this patch at all.
> > 
> > `extern "C" int __nvvm_reflect(const char *);` appears to work just fine already:
> > https://godbolt.org/z/caGenxn1e
> > 
> > 
> It does work fine unless we are using openCL, which requires the addrspace casting as mentioned above
Here's the way the situation looks to me:
* __nvvm_reflect is a technical debt (IMO) that exists because NVIDIA happened to use it in libdevice bitcode so we had to make LLVM deal with it.
* It's not intended to be used as a public API outside of libdevice.
* you want to use it in a library. Just one library, AFAICT.
* there is a way to use it, though it does take some casts (BTW, we're only checking the name of the function, so you could declare it with a `__constant` argument : https://godbolt.org/z/s9EvGfPhe)
* this CL intends to make `__nvvm_reflect` to be available in clang for wider use, which will make removing it in the future much harder.

On the balance, I still do not think that it's worth exposing `__nvvm_reflect` as a clang builtin.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137154/new/

https://reviews.llvm.org/D137154