tra added inline comments.
================ Comment at: clang/include/clang/AST/GlobalDecl.h:40 + Stub = 1, +}; + ---------------- rjmccall wrote: > tra wrote: > > rjmccall wrote: > > > The attribute here is `CUDAGlobalAttr`; should this be named in terms of > > > CUDA, or is the CUDA model sufficiently different from HIP that the same > > > implementation concept doesn't apply? > > I believe the attribute serves the same purpose in both CUDA and HIP and > > could be renamed appropriately in a separate patch. > > > > While the changes in this patch are not required for CUDA, CUDA would > > benefit from them. We could use a generic GPU prefix and migrate CUDA to > > the same model later. A TODO comment about that would be nice. > I'd just like consistency. If they're serving the same purpose, then as > someone with no dog in this fight, I would give precedence to CUDA over HIP > in names since it's both the older language and was implemented first in > Clang (even if only partially, IIUC). I don't think a generic name works > unless we can meaningfully generalize it to all languages with a similar > feature, e.g. OpenCL and so on. Naming, the hardest problem in computer science. :-) I personally would prefer generalization-with-exclusions over specific name which is inconsistently commingles things that's really specific to that name and things that are more widely used. Alas, right now CUDA is the example of the latter case -- some parts are CUDA-specific and a lot are shared with HIP. For the new features we've been sort of sticking with using CUDA/HIP for specific parts and GPU for shared functionality, but as things are a lot of shared bits are still 'CUDA' and it's hard to tell them apart. As you point it out, renaming the incumbent names would be a pain, so here we are. I think using `GPUKernelKind` with a comment that it reflects HIP & CUDA kernels would be somewhat better choice than `CUDAKernelKind` which would be somewhat confusing at this point given that CUDA actually does not use it yet. I'm also fine with keeping it `HIPKernelKind` and postpone the naming decision until CUDA-related parts are actually implemented. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68578/new/ https://reviews.llvm.org/D68578 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
