Skip to content

Conversation

@MrSidims
Copy link
Contributor

This patch should be reverted in the future as it supports translation of invalid SPIR-V modules.

This patch should be reverted in the future as it supports translation
of invalid SPIR-V modules.

Signed-off-by: Sidorov, Dmitry <[email protected]>
@MrSidims MrSidims force-pushed the llvm_release_160-wa-target branch from 9d60116 to dae4222 Compare December 18, 2025 13:18
@michalpaszkowski michalpaszkowski merged commit ee2a14e into KhronosGroup:llvm_release_160 Dec 18, 2025
9 checks passed
@MrSidims
Copy link
Contributor Author

MrSidims commented Dec 18, 2025

Sorry that this was almost silently merged. Let me put a comment for history and tag @svenvh . This is quite a nasty patch, which fortunately doesn't affect anybody, which was prepared to workaround an issue related to opaque pointers switch in https://github.com/intel/intel-graphics-compiler .

It appeared to be, that some of our frontends were incorrectly adding a pointer-to-integer cast to pass Images as integers to .genx intrinsics. It is inherently wrong (and will be fixed), but it just worked because previously all OpenCL-specific types were pointers to opaque structures (and so LLVM had no complains about such IR) and llvm-spirv was and still is accepting this (while the correct behaviour is to emit an error) (right now it's accepting it if OpConvertPtrToU is SPIR-V-friendly call in LLVM IR with target type Image 'pointer' operand).

So this patch lets incorrect SPIR-V modules generated by the translator be consumed.

@michalpaszkowski
Copy link
Member

michalpaszkowski commented Dec 18, 2025

I will prepare a proper patch to the SPIR-V Writer first, so that it catches issues like this one, then we will be able to remove this workaround here. Thanks @MrSidims for the patch and providing the context.

Please expect a patch from me in January.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants