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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits