Skip to content

[SYCL][TEST] Fix math_test_marray_vec failure on Adreno with ULP - #22867

Merged
KornevNikita merged 1 commit into
intel:syclfrom
vishwajithupendra:marray_vec_ULP
Aug 11, 2026
Merged

[SYCL][TEST] Fix math_test_marray_vec failure on Adreno with ULP#22867
KornevNikita merged 1 commit into
intel:syclfrom
vishwajithupendra:marray_vec_ULP

Conversation

@vishwajithupendra

Copy link
Copy Markdown
Contributor

The test was crashing with exit code 0xC0000409 (assertion failure) because erfc(0) on the Adreno GPU returns a value 2 ULPs away from the mathematically exact result:

GPU: 0.99999988 (0x3F7FFFFF)
Expected: 1.0 (0x3F800000)
ULP diff: 2

The root cause is that the original checkEqual overloads used exact equality (== / !=) which does not account for GPU floating-point rounding. The OpenCL 3.0 spec permits up to 16 ULPs for erfc, so the GPU result is well within spec.

A new scalar checkEqual(T a, T b, unsigned maxUlps = 0) overload is added and placed in a new shared header ulp_utils.hpp at the test-e2e root so it is available to all tests. The two existing vec<T,3> and vec<T,4> overloads are replaced by a single generic vec<T,N> overload, and the marray<T,N> overload is updated to accept and pass through the maxUlps parameter. A static_assert is added to restrict checkEqual to float, double, and sycl::half.

The ULP distance is computed by:

  1. Short-circuiting on a == b to correctly handle -0 vs +0 (IEEE 754 equal but 1 ULP apart in the bit representation).
  2. Reinterpreting float/double/half bits as uint32_t/uint64_t/uint16_t via memcpy.
  3. Remapping sign-magnitude to an ordered integer number line so that integer subtraction gives the correct ULP distance at all magnitudes and across the positive/negative boundary.

The tolerance was determined empirically by measuring the actual ULP diff for all 48 subtests on the Adreno GPU:

  • 47/48 subtests: 0 ULPs (bit-exact)
  • erfc(0): 2 ULPs (only non-exact result)

The maxUlps parameter defaults to 0 (exact equality) so all 47 bit-exact subtests are unchanged. Only the two erfc call sites (in math_tests_3 and math_tests_4) pass maxUlps = 2, making the tolerance precisely targeted to the one function that needs it.

ULP

The test was crashing with exit code 0xC0000409 (assertion failure)
because erfc(0) on the Adreno GPU returns a value 2 ULPs away
from the mathematically exact result:

  GPU:      0.99999988 (0x3F7FFFFF)
  Expected: 1.0        (0x3F800000)
  ULP diff: 2

The root cause is that the original checkEqual overloads used exact
equality (== / !=) which does not account for GPU floating-point
rounding. The OpenCL 3.0 spec permits up to 16 ULPs for erfc, so
the GPU result is well within spec.

A new scalar checkEqual(T a, T b, unsigned maxUlps = 0) overload
is added and placed in a new shared header ulp_utils.hpp at the
test-e2e root so it is available to all tests. The two existing
vec<T,3> and vec<T,4> overloads are replaced by a single generic
vec<T,N> overload, and the marray<T,N> overload is updated to accept
and pass through the maxUlps parameter. A static_assert is added to
restrict checkEqual to float, double, and sycl::half.

The ULP distance is computed by:
  1. Short-circuiting on a == b to correctly handle -0 vs +0 (IEEE 754
     equal but 1 ULP apart in the bit representation).
  2. Reinterpreting float/double/half bits as uint32_t/uint64_t/uint16_t
     via memcpy.
  3. Remapping sign-magnitude to an ordered integer number line so that
     integer subtraction gives the correct ULP distance at all
     magnitudes and across the positive/negative boundary.

The tolerance was determined empirically by measuring the actual ULP
diff for all 48 subtests on the Adreno GPU:

  - 47/48 subtests: 0 ULPs (bit-exact)
  - erfc(0):        2 ULPs (only non-exact result)

The maxUlps parameter defaults to 0 (exact equality) so all 47
bit-exact subtests are unchanged. Only the two erfc call sites
(in math_tests_3 and math_tests_4) pass maxUlps = 2, making the
tolerance precisely targeted to the one function that needs it.
@vishwajithupendra

vishwajithupendra commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@KseniyaTikhomirova @bader
Looks like the CI run is failing with a known issue (#22858) unrelated to the fix submitted here.
Can you help bypass this and let the CI move through?

@bader

bader commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

@KseniyaTikhomirova @bader Looks like the CI run is failing with a known issue (#22858) unrelated to the fix submitted here. Can you help bypass this and let the CI move through?

@vishwajithupendra, known CI issues shouldn't block the merge. This should be ready to go as soon as @intel/llvm-reviewers-runtime team approves.

I suggest dropping UPSTREAM tag from the PR title.

@KseniyaTikhomirova

Copy link
Copy Markdown
Contributor

I have a concern about this change since it relaxes pass criteria unconditionally for all targets this test is used for. That means that if another backend or HW will have a regression in accuracy comparing to the current one - we may miss it due to relaxed conditions.

@vishwajithupendra do you use opencl backend only?

@vishwajithupendra vishwajithupendra changed the title [UPSTREAM][SYCL][TEST] Fix math_test_marray_vec failure on Adreno with ULP [SYCL][TEST] Fix math_test_marray_vec failure on Adreno with ULP Aug 11, 2026
@VerenaBeckham

Copy link
Copy Markdown
Contributor

I have a concern about this change since it relaxes pass criteria unconditionally for all targets this test is used for. That means that if another backend or HW will have a regression in accuracy comparing to the current one - we may miss it due to relaxed conditions.

@vishwajithupendra do you use opencl backend only?

We do use OpenCL only, but I don't think it makes sense to complicate (or duplicate) a lot of tests (because there are plenty more) by enforcing floating point equality "just in case". In the end, these are tests to check SYCL functionality, which allows at least some ULP of difference (depending on the backend), e.g. the CUDA backend allows 5 ULPs. These are not tests to check the accuracy of the HW you're running on.

@KseniyaTikhomirova KseniyaTikhomirova left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@KseniyaTikhomirova

Copy link
Copy Markdown
Contributor

@intel/llvm-gatekeepers please consider merging this. This PR affects only specific group of tests, failure in
bindless_images/vulkan_interop/vulkan_sycl_image_interop_write_1d_unsampled.cpp can't be related

@KornevNikita

Copy link
Copy Markdown
Contributor

known failures, merging

@KornevNikita
KornevNikita merged commit 48737ca into intel:sycl Aug 11, 2026
29 of 31 checks passed
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.

5 participants