hliao marked 3 inline comments as done.
hliao added inline comments.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7719
+ // Arrary types.
+ if (auto ATy = dyn_cast<llvm::ArrayType>(Ty)) {
+ auto T = ATy->getElementType();
----------------
hliao wrote:
> hliao wrote:
> > arsenm wrote:
> > > No tests with arrays or structs?
> > >
> > > It's also not immediately obvious to me that this optimization is still
> > > valid if the pointer is buried in a struct
> > the original generic kernel pointer promotion to a global one only handles
> > the pointer directly passed. From a critical workload, I found quite a few
> > cases where the global pointers are passed through a by-val struct. We
> > didn't handle that yet. With this case, we could start to handle that.
> struct tests are added. From test cases, it seems to me that arry is not
> passed by value. I need to double-confirm.
a test case for arrary types is added.
================
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 {
----------------
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.
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
