jdoerfert added a comment.

In D84743#2181044 <https://reviews.llvm.org/D84743#2181044>, @MaskRay wrote:

> In D84743#2181031 <https://reviews.llvm.org/D84743#2181031>, @jdoerfert wrote:
>
>> In D84743#2179441 <https://reviews.llvm.org/D84743#2179441>, @tra wrote:
>>
>>> I'm not sure it's particularly useful, to be honest. CUDA code still needs 
>>> to be compatible with NVCC so it can't be used in portable code like TF or 
>>> other currently used CUDA libraries.
>>> It could be useful internally, though, so I'm fine with it for that purpose.
>>
>> FWIW, I was only thinking about `clang/lib/Header` usage. *Potentially* 
>> documented for user of clang.
>
> Honestly I am a bit uneasy with the new clang/lib/Header file. It will be 
> part of the clang resource directory and users on every target will be able 
> to `#include <offload_macros.h>` it.
> This is also a namespace pollution - used incorrectly, people can trip over 
> it if they have files of the same name.

There are two levels to it though. 1) `clang/lib/Header`, and 2) 
`clang/lib/Header/XXXXXX`. We do not expose XXXXX to every user on every 
target, so when we do we really want the headers to be first. So for 2) the 
names should match existing system headers we want to wrap. For 1) the header 
names should be "in the compiler namespace". That said, the file above is not 
and I didn't notice before. The existing CUDA overloads that live in 1) start 
with `__cuda`, which should be sufficient for users not to trip over them. I 
mean, they could trip over a lot of things that starts with `__`. I imagine we 
have a `__gpu_...` set of header soon to avoid duplicating or polluting the 
`__cuda` ones (more). Now that I finished all this, also the `XXXXXX` above 
needs to be renamed into `__XXXXX`, but that seems easy enough to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84743

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to