Skip to content

Adjust hyperbolic trig functions to use ULP rules #342

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

Merged
merged 7 commits into from
Aug 12, 2025

Conversation

llvm-beanz
Copy link
Collaborator

@llvm-beanz llvm-beanz commented Aug 10, 2025

Unlike the standard trig functions which may be implemented as natural calculations or as hyperbolic aproximations, the hyperbolic functions are guaranteed to be hyperbolc aproximations and thus have consistent precision characteristics.

These can all be guaranteed within 2 ULP, which gives a better match and will resolve the test failures on AMD GPUs.

The cosh.16 test is not updated in this PR because it is already using ULP rules and 2 ULP range specification.

I've captured a spec issue to follow up on the very wide range requirements for tanh on NVIDIA GPUs:
microsoft/hlsl-specs#601

This issue has also been observed by WGSL and is reflected in their spec:
gpuweb/gpuweb#5199

Fixes #326

Unlike the standard trig functions which may be implemented as natural
calculations or as hyperbolic aproximations, the hyperbolic functions
are guaranteed to be hyperbolc aproximations and thus have consistent
precision characteristics.

These can all be guaranteed within 2 ULP, which gives a better match and
will resolve the test failures on AMD GPUs.

The `cosh.16` test is not updated in this PR because it is already using
ULP rules and 2 ULP range specification.
@llvm-beanz llvm-beanz requested a review from spall August 10, 2025 20:31
@llvm-beanz llvm-beanz added the test-all When applied to a PR this will opt-in to additional pre-merge test configurations.. label Aug 10, 2025
@bob80905
Copy link
Contributor

Do we want -Gis to be added as an option to each test?

@llvm-beanz
Copy link
Collaborator Author

Do we want -Gis to be added as an option to each test?

Yea, we probably need to for any test that covers NaN behavior. I think we need to figure out some issues with DXC and Clang for how we handle math flags to make sure that -Gis does what we need.

@llvm-beanz llvm-beanz merged commit 19464d2 into llvm:main Aug 12, 2025
9 of 18 checks passed
@llvm-beanz
Copy link
Collaborator Author

Do we want -Gis to be added as an option to each test?

Yea, we probably need to for any test that covers NaN behavior. I think we need to figure out some issues with DXC and Clang for how we handle math flags to make sure that -Gis does what we need.

I filed a follow-up issue to this and assigned it to myself: #348

Once we figure out what needs to be done for SPIRV support I'll go through and cleanup the existing tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-all When applied to a PR this will opt-in to additional pre-merge test configurations..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test Feature/HLSLLib/sinh.16.test fails under windows-amd for build targets check-hlsl-vk and check-hlsl-clang-vk
4 participants