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

[Ronan Keryell via Phabricator via cfe-commits]

keryell added inline comments.


================
Comment at: clang/lib/Basic/Targets/SPIR.h:59
+    // translation). This mapping is enabled when the language mode is HIP.
+    1, // cuda_device
+    // cuda_constant pointer can be casted to default/"flat" pointer, but in
----------------
Anastasia wrote:
> bader wrote:
> > Anastasia wrote:
> > > I am slightly confused as in the LLVM project those address spaces are for SPIR not SPIR-V though. It is however used outside of LLVM project by some tools like SPIRV-LLVM Translator as a path to SPIR-V, but it has only been done as a workaround since we had no SPIR-V support in the LLVM project yet. And if we are adding it let's do it clean to avoid/resolve any confusion.
> > > 
> > > I think we need to keep both because some vendors do target/use SPIR but not SPIR-V.
> > > 
> > > So if you are interested in SPIR-V and not SPIR you should probably add a new target that will make things cleaner.
> > > I think we need to keep both because some vendors do target/use SPIR but not SPIR-V.
> > 
> > @Anastasia, could you elaborate more on the difference between SPIR and SPIR-V?
> > I would like to understand what these terms mean in the context of LLVM project.
> Their conceptual differences are just that they are two different intermediate formats.
> 
> The important thing to highlight is that it is not impossible that some vendors use SPIR (without using SPIR-V) even despite the fact it has been discontinued by Khronos. 
> 
> Nobody has deprecated or discontinued SPIR in the LLVM project yet.
> Their conceptual differences are just that they are two different intermediate formats.
> 
> The important thing to highlight is that it is not impossible that some vendors use SPIR (without using SPIR-V) even despite the fact it has been discontinued by Khronos. 
> 
> Nobody has deprecated or discontinued SPIR in the LLVM project yet.

All the official Xilinx OpenCL stack is based on legacy SPIR (encoded in LLVM 6.x IR but this is another story) and I suspect this is the case for other companies.
So, do not deprecate or discontinue, please. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621