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

Do not check for undefined behavior in abs. #831

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

hvdijk
Copy link
Contributor

@hvdijk hvdijk commented Nov 8, 2023

Unlike OpenCL abs, but like C and C++ abs, in SYCL 2020, taking the absolute value of a signed type produces a signed result and the behavior is undefined if the absolute value is not representable (i.e. if the input is the greatest negative value), so do not check that the SYCL results match the reference results for this case.

@hvdijk hvdijk requested a review from a team as a code owner November 8, 2023 12:50
@bader bader added the help wanted Extra attention is needed label Nov 8, 2023
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.

Almost there.

util/math_reference.h Outdated Show resolved Hide resolved
Unlike OpenCL abs, but like C and C++ abs, in SYCL 2020, taking the
absolute value of a signed type produces a signed result and the
behavior is undefined if the absolute value is not representable (i.e.
if the input is the greatest negative value), so do not check that the
SYCL results match the reference results for this case.
@hvdijk
Copy link
Contributor Author

hvdijk commented Nov 17, 2023

ping @keryell

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!

@bader bader merged commit 57cc89d into KhronosGroup:SYCL-2020 Nov 17, 2023
7 checks passed
@hvdijk hvdijk deleted the abs branch November 17, 2023 23:49
steffenlarsen added a commit to steffenlarsen/SYCL-CTS that referenced this pull request Dec 7, 2023
Similar to sycl::abs, sycl::abs_diff now has undefined behavior for
unrepresentable results. This patch changes the generated tests for
abs_diff to avoid UB, similar to how it was done for abs in
KhronosGroup#831.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
steffenlarsen added a commit to steffenlarsen/SYCL-CTS that referenced this pull request Dec 7, 2023
Similar to sycl::abs, sycl::abs_diff now has undefined behavior for
unrepresentable results. This patch changes the generated tests for
abs_diff to avoid UB, similar to how it was done for abs in
KhronosGroup#831.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants