Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop precision requirements for host math built-ins #859

Conversation

AlexeySachkov
Copy link
Contributor

KhronosGroup/SYCL-Docs#511 essentially removed any precision guarantees for math built-ins invoked on host and this PR aligns CTS with that direction.

KhronosGroup/SYCL-Docs#511 essentially removed any precision guarantees for
math built-ins invoked on host and this PR aligns CTS with that
direction.
@AlexeySachkov AlexeySachkov requested a review from a team as a code owner February 1, 2024 12:33
Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Thanks.

@AerialMantis
Copy link
Collaborator

SYCL WG call:

  • One approval, looks good to merge.

@bader bader merged commit 3197f43 into KhronosGroup:SYCL-2020 Feb 1, 2024
8 checks passed
@hvdijk
Copy link
Contributor

hvdijk commented Feb 8, 2024

Hi, is it intentional that this still checks for inf vs. non-inf? With DPC++ commit 557df1aa57c810732f1635abb85045f407c5d0f3, SYCL-CTS commit 3197f43, I'm seeing, for instance,

/home/harald/SYCL-CTS/tests/math_builtin_api/math_builtin.h:196: FAILED:
  CHECK( verify(log, hostRes, ref, -1, comment) )
with expansion:
  false
with message:
  tests case: 4000523. Correctness check failed on host.

/home/harald/SYCL-CTS/util/logger.cpp:41: warning:
  value: inf [7f800000], reference: 0 [0]

/home/harald/SYCL-CTS/util/logger.cpp:41: warning:
  Expected accuracy in ULP: -1

for the test case

{

        sycl::marray<float, 5> inputData_0(0.1958509433f,0.2681012382f,0.2765018591f,0.2239653187f,0.5127045541f);
        int inputData_1(7348);

        sycl_cts::resultRef<sycl::marray<float, 5>> ref = reference::ldexp(inputData_0,inputData_1);

  check_function<4000523, sycl::marray<float, 5>>(log,
      [=]{

        sycl::marray<float, 5> inputData_0(0.1958509433f,0.2681012382f,0.2765018591f,0.2239653187f,0.5127045541f);
        int inputData_1(7348);

        static_assert(std::is_same_v<decltype(sycl::ldexp(inputData_0, inputData_1)), sycl::marray<float, 5>>,
            "Error: Wrong return type of sycl::ldexp(sycl::marray<float, 5>, int), not sycl::marray<float, 5>");

        static_assert(std::is_same_v<decltype(sycl::ldexp<decltype(inputData_0)>(inputData_0, inputData_1)), decltype(sycl::ldexp(inputData_0, inputData_1))>,
            "Error: sycl::ldexp(sycl::marray<float, 5>, int) definition does not use the required template arguments.");

        return sycl::ldexp(inputData_0, inputData_1);

      }, ref, 0);
}

@AlexeySachkov
Copy link
Contributor Author

@hvdijk, that is a good question and I'm not sure if I know the right answer.

Strictly speaking, the SYCL 2020 specification poses no requirements on host built-ins precision and therefore there are no guarantees about the result. However, I would expect that an implementation would return at least somewhat reasonable result and therefore I left checks for inf/non-inf in place.

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