[PATCH] D155978: [SPIRV] Add SPIR-V logical triple.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307
+  EXPECT_TRUE(T.isSPIRV());
+
   T.setArch(Triple::spirv32);
----------------
pmatos wrote:
> Keenuts wrote:
> > pmatos wrote:
> > > I am not well-versed in SPIRV for gfx but if we are using 32bits in SPIRV logical, can't we reuse spirv32? Is there some sort of strong reason not to or adding spirv for logical spirv as an alternative to spirv32 and spirv64 is just easier?
> > This is a very valid question! And I'm not sure of the best option.
> > My understanding is: in logical SPIR-V, there are no notion of pointer size, or architecture size. We cannot offset pointers by a sizeof(), or do such things.
> > 
> > But the options I had didn't seem to allow this kind of "undefined architecture".
> > I chose 32bits here because the IDs are 32bit. But pointer addresses are not, so returning 32bit is not quite right either.
> > 
> > 
> This is a tricky one tbh - I would have to do a bit more research to have a more insighful reply here. Maybe @mpaszkowski or @Anastasia can add more.
I think to do this properly in LLVM would require an IR extension or something, it is maybe worth starting RFC to discuss this.

Out of curiosity do you have any code that can be compiled from any GFX source language that would need a pointer size to be emitted in IR? If there is no code that can be written in such a way then using one fixed pointer width could be ok. Then the SPIR-V backend should just recognise it and lower as required. Otherwise target configurable types might be useful: https://llvm.org/docs/LangRef.html#target-extension-type

In general we tried to avoid adding bitwidth neutral SPIR-V triple originally just because LLVM always requires a pointer width. We were thinking of adding vulkan as a component to the existing SPIR-V 34|64 targets https://discourse.llvm.org/t/setting-version-environment-and-extensions-for-spir-v-target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155978