[PATCH] D68578: [HIP] Fix device stub name

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/include/clang/AST/GlobalDecl.h:40
+  Stub = 1,
+};
+
----------------
rjmccall wrote:
> 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.
SGTM.


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

https://reviews.llvm.org/D68578