tra added inline comments.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7689
+ // Coerce HIP pointer arguments from generic pointers to global ones.
+ llvm::Type *coerce(llvm::Type *Ty, unsigned DefaultAS,
+ unsigned GlobalAS) const {
----------------
hliao wrote:
> tra wrote:
> > Now it could use a more descriptive name, too. :-)
> >
> > You can now also make DefaultAS/GlobalAS into local variables as you have
> > access to `getContext()` here.
> name is changed but I want to leave `DefaultAS` and `GlobalAS` as parameters
> as they may vary from HIP to OpenCL and different targets. Even though it may
> be rare case, I want to avoid careless errors.
You may not need it, ever and it would be easy to add, but I'll leave it up to
you.
If you do want to keep them as parameters you may want to consider renaming
them to FromAS/ToAS.
There's nothing in the code that has anything to do with whether they are for
generic/specific address space and the function name does not indicate the
direction of coercion between them. It's very easy to pass them in the wrong
order and not notice it. Making them local variables would avoid it. Giving
names some sort of 'directionality' would at least give user a hint what goes
where, even if it would not prevent making the error.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69826/new/
https://reviews.llvm.org/D69826
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
