bader 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
----------------
keryell wrote:
> 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. :-)
> 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.

Strictly speaking `SPIR` is not defined as an intermediate language. Khronos 
defines `SPIR-1.2` and `SPIR-2.0` formats which are based on LLVM 3.2 and LLVM 
3.4 version (https://www.khronos.org/spir/). There is no definition of SPIR 
format based on current version of LLVM IR. Another note is that metadata and 
intrinsics emitted for OpenCL with clang-14 doesn't follow neither `SPIR-1.2` 
nor `SPIR-2.0`.

I always think of LLVM IR as leaving thing that is subject to change by LLVM 
community, so tools working with LLVM IR must adjust to the particular version 
(e.g. release version like LLVM 13 or ToT). We apply this logic to 
SPIRV-LLVM-Translator tool and update it according to LLVM format changes (e.g. 
kernel argument information defined in Khronos spec must be named metadata 
whereas clang emits function metadata).

> I am slightly confused as in the LLVM project those address spaces are for 
> SPIR not SPIR-V though.
[skip]
> Their conceptual differences are just that they are two different 
> intermediate formats.

If this is the only difference, I don't think it a good idea to create another 
LLVM target to separate SPIR and SPIR-V. From my point of view it creates logic 
ambiguity and code duplication with no additional value. @Anastasia, what 
problems do you see if we continue treating LLVM IR with spir* target triple as 
LLVM IR representation of SPIR-V format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to