Skip to content

[SYCL] tighten known_identity to finite_math macro #18247

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

Open
wants to merge 3 commits into
base: sycl
Choose a base branch
from

Conversation

cperkinsintel
Copy link
Contributor

@cperkinsintel cperkinsintel commented Apr 29, 2025

Some time ago we fixed a bug in sycl::known_identity where it was incorrectly using std::numeric_limits<T>::infinity() even in cases where ::infinity() should not have been available. At the time, the fix was to simply test for __FAST_MATH__.

That worked, but was not ideal. Now we are tightening this to use the __FINITE_MATH_ONLY__ macro which is more accurate.
see #13813

__FINITE_MATH_ONLY__ will be set when both the -fno-honor-infinities and -fno-honor-nans flags are used. It was felt by the FE team that there is no reasonable case where one of the flags would be used without the other and therefore __FINITE_MATH_ONLY__ is sufficient for this problem ( and superior to the __FAST_MATH__ macro we were using before).

Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel cperkinsintel marked this pull request as ready for review April 30, 2025 19:12
@cperkinsintel cperkinsintel requested a review from a team as a code owner April 30, 2025 19:12
@aelovikov-intel
Copy link
Contributor

see CMPLRLLVM-58728, CMPLRLLVM-65507

This isn't approved usage for referencing internal bug reports. We are only supposed to do that when we can't share information there, e.g. issues in underlying proprietary layers. Anything concerning SYCL RT or C/C++ FE is public and should be accessible for anybody reading this PR/commit message.

@cperkinsintel
Copy link
Contributor Author

@aelovikov-intel - I encouraged everyone to use issue #13813 for technical discussions, but some of it was conducted on the JIRAS. This particular PR is pretty simple, I don't have anything to add to the description or the comment.

@aelovikov-intel
Copy link
Contributor

Please summarize relevant bits from these internal bug-reports directly in the PR description and remove references to the jiras.

Signed-off-by: Chris Perkins <[email protected]>
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.

3 participants