[PATCH] D109818: [HIPSPV] Convert HIP kernels to SPIR-V kernels

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10224
+    // pointers for HIPSPV. When the language mode is HIP, the SPIRTargetInfo
+    // maps cuda_device to SPIR-V's CrossWorkGroup address space.
+    llvm::Type *LTy = CGT.ConvertType(Ty);
----------------
Anastasia wrote:
> linjamaki wrote:
> > Anastasia wrote:
> > > Can you explain why this mapping is needed? We already have an address space map to perform the mapping of address spaces b/w language and target. It would be good if we don't replicate similar logic in too many places.
> > HIP does not require address space qualifiers on kernel pointer arguments (e.g. see hipspv-kernel.cpp) nor there are AS qualifiers that can be placed on them. With the default logic, provided by SPIRVTargetInfo’s address space map, the kernel pointer arguments get converted to generic pointers which are not allowed by the OpenCL SPIR-V Environment Specification.
> I feel that it is the same for SYCL... It might be good to check with @bader whether there is already a way to handle this that can be reused for HIP...
We need to do similar transformation for SYCL, but it's not exactly the same. 
For SYCL kernels, which represented as function objects, compiler generates SPIR kernel function and fixes up the address space for pointer arguments in compiler generated declaration. For more details, see the description of https://reviews.llvm.org/D71016  and `handlePointerType` function code in clang/lib/Sema/SemaSYCL.cpp of this review request (lines 848-876). As address space is fixed in Sema, it works for all targets SYCL currently supports SPIR, NVPTX and AMDGPU.

If I understand it correctly, we are trying to do minimal amount of work for convert HIP kernel function to SPIR kernel function, i.e. fix calling convention and address spaces. 
Are these two fixes enough or we need more fixes to enable more sophisticated kernels?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109818/new/

https://reviews.llvm.org/D109818