[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

Mon Sep 13 04:40:42 PDT 2021

linjamaki added inline comments.

================
Comment at: clang/lib/Basic/Targets/SPIR.h:233
+    if (Opts.HIP && Opts.CUDAIsDevice)
+      // Enable address space mapping from HIP to SPIR-V.
+      // See comment on the SPIRDefIsGenMap table.
----------------
Anastasia wrote:
> My guess is that this is not only HIP specific but for example the same applies to SYCL.
> 
> I am not sure if it makes more sense to move this into a `BaseSPIRTargetInfo` since this is not really SPIR-V specific logic. It is just a clang design misalignment between two address space concepts that has to be addressed properly at some point.
> 
The DefaultIsGeneric AS mapping is enabled for SYCL in the BaseSPIRTargetInfo::adjust (which also means the mapping is available for both the SPIR and SPIR-V targets). On the other hand, the AS mapping for HIPSPV is enabled in SPIRVTargetInfo::adjust only as we intend to emit SPIR-V only. I’m under the impression that this is what was wanted.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621