rjmccall added inline comments.
================ Comment at: clang/include/clang/AST/GlobalDecl.h:40 + Stub = 1, +}; + ---------------- tra wrote: > 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. Maybe `KernelReferenceKind`? It's probably a common concept across all heterogenous-computing models. 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
