rjmccall added inline comments.
================
Comment at: clang/include/clang/AST/GlobalDecl.h:40
+ Stub = 1,
+};
+
----------------
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.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1028
else
- MC.mangleName(ND, Out);
+ MC.mangleName(GD, Out);
} else {
----------------
tra wrote:
> 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.
Yes, that's a good idea.
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
