[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:10221
+ABIArgInfo SPIRABIInfo::classifyKernelArgumentType(QualType Ty) const {
+  if (getContext().getLangOpts().HIP && getTarget().getTriple().isSPIRV()) {
+    // Coerce pointer arguments with default address space to CrossWorkGroup
----------------
Anastasia wrote:
> It feels like this needs to be in `SPIRVABIInfo`  or something? Or can this be generalized to both  - SPIR and SPIR-V?
A comment was added in D109144 to state that the SPIRABIInfo is an ABI implementation for both the SPIR and SPIR-V. For now, there is not much difference between SPIR and SPIR-V for this class. Would it be satisfactory if the class is renamed to something more general (like CommonSPIRABIInfo)?


================
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:
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109818