[PATCH] D111566: [SYCL] Fix function pointer address space
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 1 18:09:24 PST 2021
rjmccall added inline comments.
Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:636-638
+ unsigned AS = PointeeType->isFunctionTy()
+ ? getDataLayout().getProgramAddressSpace()
+ : Context.getTargetAddressSpace(ETy);
> eandrews wrote:
> > aaron.ballman wrote:
> > > The review summary says that this is a fix for SYCL, but the fix itself happens for all targets, not just SYCL. If that's intentional, are we sure it's correct?
> > Yes this affects all targets. To be honest, I'm not sure if this change is correct for CUDA/openCL, etc. My first patch (which I didn't upload) restricted the change to SYCL. However, I saw the same thing was done in a generic manner for function pointers - https://github.com/llvm/llvm-project/commit/57fd86de879cf2b4c7001b6d0a09df60877ce24d, and so followed the same logic. I'm hoping reviewers more familiar with address spaces can help here.
> @Anastasia -- can you comment as OpenCL code owner?
I think the more systematic fix is probably for `getTargetAddressSpace` to check for function types and return the program address space, yeah.
CHANGES SINCE LAST ACTION
More information about the cfe-commits