tra added a comment.

Nice! Thank you for making these changes.



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


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1028
     else
-      MC.mangleName(ND, Out);
+      MC.mangleName(GD, Out);
   } else {
----------------
rjmccall wrote:
> Let's see if we can make this breakdown no longer necessary, since 
> `MangleContext::mangleName` should be capable of doing the right thing 
> starting straight from a GD.  In fact, maybe we can remove most of the 
> specialized mangling methods (like  `mangleCXXCtor` and `mangleCXXDtor`) from 
> `MangleContext` completely?
> 
> Unrelatedly: there's an `Out` declared in the outermost scope, but a bunch of 
> these scopes declare their own `Out`; could you just fix that while you're 
> editing this function?
Perhaps it would make sense to split this patch into two -- one that changes 
mangler input to GlobalDecl and the other one dealing with HIP stubs.


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

Reply via email to