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

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki 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);
----------------
bader wrote:
> 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?
This patch and D108621 covers only the calling convention and the address space aspects in HIP->SPIR-V conversion. There are still various HIP features [1] which need to be expanded or emulated afterwards. A fully linked device program is needed before the fixes can be applied, so these fixes won’t be implemented at Sema nor CodeGen which operate per translation unit. The full-program fixes are applied by the HIPSPV tool chain by applying LLVM passes provided by a HIPSPV runtime [2].  For the time being, we are not seeing a need to specialize SPIR target infos further.

[1]: “HIP code expansion” section of the “[RFC][HIPSPV] Emitting HIP device code as SPIR-V”
[2]: clang/lib/Driver/ToolChains/HIPSPV.cpp:constructEmitSpirvCommand() @ https://github.com/parmance/llvm-project/tree/hipspv-wip


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109818