Keenuts added inline comments.
================ Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307 + EXPECT_TRUE(T.isSPIRV()); + T.setArch(Triple::spirv32); ---------------- Anastasia wrote: > 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. Hello! Thanks for the feedback! I asked around, and it seems that no, we cannot write code that pointer arithmetic or requires the pointer size that I know of. The code that could require is changes the memory modal to something else, like this proposal: https://github.com/microsoft/hlsl-specs/blob/main/proposals/0010-vk-buffer-ref.md But here, the memory model is PhysicalStorageBuffer64, which tells us pointers are 64bit. We also have the SPV_KHR_variable_pointers extension (https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_variable_pointers.asciidoc) which specifically says pointers have no size or bit pattern... > Otherwise target configurable types might be useful Not familiar enough with LLVM/Clang to fully understand, so reformulating we'd have: - the target would return 32-bit, even if we have no pointer size - the IR would not use classic pointers at all, since their type would be misleading. - the IR would use this newly created type (let's call is LogicalPointer) instead of real pointers. - the backend would then convert this new type to something else when lowering. If that's the case, seems fair, I'd follow this advice. > We were thinking of adding vulkan as a component to the existing SPIR-V 34|64 > targets Same idea here, add VulkanX to the OSType part of the triple (replacing the ShaderModel used for DXIL). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155978/new/ https://reviews.llvm.org/D155978 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
