jhuber6 added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:1198
 
-def CUDAGlobal : InheritableAttr {
-  let Spellings = [GNU<"global">, Declspec<"__global__">];
+def CUDAGlobal : InheritableAttr, TargetSpecificAttr<TargetNVPTX> {
+  let Spellings = [GNU<"global">, Declspec<"__global__">, 
Clang<"nvptx_kernel">];
----------------
shangwuyao wrote:
> jhuber6 wrote:
> > tra wrote:
> > > Nice.
> > > 
> > > This reminded me that we have a project compiling CUDA, but targeting 
> > > SPIR-V instead of NVPTX. It looks like this will likely break them. The 
> > > project is out-of-tree, but I'd still need to figure out how to keep them 
> > > working.  I guess it would be easy enough to expand TargetNVPTX to 
> > > TargetNVPTXOrSpirV. I'm mostly concerned about logistics of making it 
> > > happen without disruption.
> > > 
> > > 
> > This might've broken more stuff after looking into it, I forgot that 
> > `AMDGPU` still uses the same CUDA attributes, and the host portion of CUDA 
> > also checks these. It would be nice if there was a way to say "CUDA" or 
> > "NVPTX", wondering if that's possible in the tablegen here.
> What's the plan here for keeping the SPIR-V and AMDGPU working? Would it work 
> if we simply get rid of the `TargetSpecificAttr<TargetNVPTX>`?
Yeah, it would I'll need to update the patch. The best solution would be if 
there were a way to say "TargetNVPTX or LangOpts.CUDA". Not sure if that's 
possible in Tablegen. The previous diff I had worked fine, but we should 
definitely try to avoid rework.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140226

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

Reply via email to