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.

Sign up for GitHub

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

Jump to bottom

determine and fix the conditions when the math brute force "divide_cr" test should be run #1901

Closed
bashbaug opened this issue Feb 26, 2024 · 2 comments · Fixed by #1997
Closed

determine and fix the conditions when the math brute force "divide_cr" test should be run #1901

bashbaug opened this issue Feb 26, 2024 · 2 comments · Fixed by #1997
Labels
mobica-backlog Issue approved by WG for Mobica to work on

Comments

@bashbaug
Copy link
Contributor

Currently, the math brute force "divide_cr" test checks whether CL_FP_CORRECTLY_ROUNDED_DIVIDE_SQRT is in the query result for CL_DEVICE_SINGLE_FP_CONFIG. If it is, it runs the divide_cr test for 32-bit float types and 64-bit double types.

With the latest code in the fp16-staging branch, the divide_cr test is additionally run for 16-bit half types.

Is this correct?

Notes:

  • The minimum accuracy for an fp64 and fp16 divide is already 0 ULP for the full profile, without passing any special build options. It is 3 ULP for fp64 and 1 ULP for fp16 for the embedded profile, though.
  • The name of the build option is -cl-fp32-correctly-rounded-divide-sqrt, and the description refers only to "single precision floating-point" specifically, so it is not clear this option should apply to fp64 or fp16 regardless.
  • I found this issue while investigating "divide_cr", but the same questions apply to "sqrt_cr" also.
@bashbaug bashbaug added the mobica-triage Issue proposed for addition to Mobica's backlog (needs WG approval) label Feb 26, 2024
@svenvh
Copy link
Member

svenvh commented Mar 18, 2024

In #1681 (comment) you already mentioned that "the safest thing to do would be to disable the divide_cr test for fp16 while we figure out what to do with it". With all the reasons you have given above, I'd argue that we should skip the divide_cr and sqrt_cr tests for fp16 types on the fp16-staging branch, and in addition we should also skip the divide_cr and sqrt_cr tests for fp64 types on the main branch.

@bashbaug
Copy link
Contributor Author

Moving to mobica-backlog, as discussed in the March 26th teleconference.

At the moment, this should probably be done as a PR to the fp16-staging branch, since it will be part of the fp16 fixes.

@bashbaug bashbaug added mobica-backlog Issue approved by WG for Mobica to work on and removed mobica-triage Issue proposed for addition to Mobica's backlog (needs WG approval) labels Mar 26, 2024
@svenvh svenvh mentioned this issue Jul 2, 2024
Open
svenvh added a commit to svenvh/OpenCL-CTS that referenced this issue Jul 2, 2024
@svenvh
math_brute_force: only test correctly rounded divide/sqrt for fp32
1b7a44d 
Skip the correctly rounded divide (divide_cr) and sqrt (sqrt_cr)
tests for fp16 and fp64.

The corresponding build option to enable correctly rounded divide and
sqrt is named `-cl-fp32-correctly-rounded-divide-sqrt` and the
description refers only to "single precision floating-point", so this
option should not apply to fp16 or fp64.

The specification states that fp16 and fp64 divide and sqrt must be
correctly rounded for the full profile, without needing any additional
build options.  This is already tested by the regular divide and sqrt
tests.  For the embedded profile the ULP requirement is non-zero, but
there is no build option to request a correctly rounded implementation
anyway.

Fixes KhronosGroup#1901 .
Relates to KhronosGroup#1996 .

Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>
@svenvh svenvh mentioned this issue Jul 2, 2024
Merged
svenvh added a commit to svenvh/OpenCL-CTS that referenced this issue Jul 30, 2024
@svenvh
math_brute_force: only test correctly rounded divide/sqrt for fp32
4777d6f 
Skip the correctly rounded divide (divide_cr) and sqrt (sqrt_cr)
tests for fp16 and fp64.

The corresponding build option to enable correctly rounded divide and
sqrt is named `-cl-fp32-correctly-rounded-divide-sqrt` and the
description refers only to "single precision floating-point", so this
option should not apply to fp16 or fp64.

The specification states that fp16 and fp64 divide and sqrt must be
correctly rounded for the full profile, without needing any additional
build options.  This is already tested by the regular divide and sqrt
tests.  For the embedded profile the ULP requirement is non-zero, but
there is no build option to request a correctly rounded implementation
anyway.

Fixes KhronosGroup#1901 .
Relates to KhronosGroup#1996 .

Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
mobica-backlog Issue approved by WG for Mobica to work on
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

math_brute_force: only test correctly rounded divide/sqrt for fp32 svenvh/OpenCL-CTS
2 participants
@svenvh @bashbaug