[SYCL] Fix bindless image sample instruction generation#21345
[SYCL] Fix bindless image sample instruction generation#213450x12CC wants to merge 2 commits intointel:syclfrom
Conversation
Signed-off-by: Michael Aziz <michael.aziz@intel.com>
Signed-off-by: Michael Aziz <michael.aziz@intel.com>
|
@slawekptak, friendly ping. |
| template <typename RetType, typename SmpImageT, typename CoordT> | ||
| static RetType __invoke__ImageReadLod(SmpImageT SmpImg, CoordT Coords, | ||
| float Level) { | ||
| // The result type of the SPIR-V instruction OpImageSampleExplicitLod must be |
There was a problem hiding this comment.
This requirement is also true for OpImageFetch. This change must be applied to all uses of that SPIRV Op.
Moreover, this change and the above suggested change will apply to many of the other "invoke" functions here since they all map to the same OpImageSampleExplicitLod and OpImageFetch.
And as discussed in this issue, #21366, the OpenCL restriction of requiring 4-vec returns or writes for unsampled images implies that DPC++ should also generate SPIR-V that adheres to that restriction, despite it not being stated in the OpenCL env spec! Nearly every invocation in this file will need to adhere to this requirement.
| template <typename RetType, typename SmpImageT, typename CoordT> | ||
| static RetType __invoke__ImageReadLod(SmpImageT SmpImg, CoordT Coords, | ||
| float Level) { | ||
| // The result type of the SPIR-V instruction OpImageSampleExplicitLod must be |
There was a problem hiding this comment.
As an aside, this file has piled up technical debt where many different __invoke__ funcs all make use of many different __spirv__ funcs which then all map to a few actual SPIR-V Ops, alongside some confusing naming being present (fetch vs read).
AFAICT, there are really only 4 different distinct SPIR-V operations being generated here:
OpImageRead__invoke__ImageRead__invoke__ImageArrayRead__invoke__ImageFetch__invoke__ImageArrayFetch
OpImageWrite__invoke__ImageWrite__invoke__ImageArrayWrite
OpImageFetch__invoke__SampledImageFetch__invoke__SampledImageArrayFetch
OpSampleExplicitLod__invoke__ImageReadLod__invoke__ImageReadGrad__invoke__ImageReadSampler__invoke__ImageReadCubemap
The current design encodes the distinction between image operands and image types in the function name resulting in 10+ __spirv_ declarations all collapsing to the same 4 SPIR-V Ops. Do you think it'd be worth cleaning this up? I can also see bindless_images.hpp adds to this confusion by generating different invocations depending on whether __SPIR__ is defined or not.
2 participants
The SPIR-V specification requires the result type of the
OpImageSampleExplicitLodinstruction to be a vector of four components. The SYCL bindless images extension allows the user to specify a return type for thesample_imagecall that may not be a vector. This commit ensures the SYCL headers correctly convert between the user-specified type and the type required by the SPIR-V specification to prevent generating invalid SPIR-V modules.