Skip to content

Conversation

0x12CC
Copy link
Contributor

@0x12CC 0x12CC commented Sep 17, 2025

The implementation previously used __spirv_ImageSampleExplicitLod which returned incorrect results when the DataT return type was a vector of floats. This change makes it use __spirv_ImageRead instead and adds a test case for the float image type.

Signed-off-by: Michael Aziz <michael.aziz@intel.com>
@0x12CC 0x12CC requested a review from a team as a code owner September 17, 2025 01:59
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

// fetch_image should not use the sampler
if constexpr (detail::is_recognized_standard_type<DataT>()) {
return FETCH_SAMPLED_IMAGE(
return FETCH_UNSAMPLED_IMAGE(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note:

  1. Some of these macros seem a little unnecessary, i.e. some of the just wrap the same function in all paths, which is in turn no longer than the macro itself.
  2. The macros bleed into user-space, i.e. we don't undef them at the end of the header. We should probably address that. Maybe we should move away from macros and use inline or anonymous functions instead?

Would you mind opening a follow-up fix for this and/or open an issue?

Copy link
Contributor

@intel/llvm-gatekeepers please consider merging

@AlexeySachkov AlexeySachkov merged commit ec607fb into intel:sycl Sep 18, 2025
44 of 45 checks passed
@0x12CC 0x12CC deleted the fix_fetch_image_float branch September 18, 2025 13:35
YixingZhang007 pushed a commit to YixingZhang007/llvm that referenced this pull request Sep 22, 2025
The implementation previously used `__spirv_ImageSampleExplicitLod`
which returned incorrect results when the `DataT` return type was a
vector of floats. This change makes it use `__spirv_ImageRead` instead
and adds a test case for the `float` image type.

Signed-off-by: Michael Aziz <michael.aziz@intel.com>
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.

3 participants