Skip to content

Conversation

wenju-he
Copy link
Contributor

0x7ff0000000000000 is +inf. Change it to quiet nan 0x7ff8000000000000.

0x7ff0000000000000 is +inf. Change it to quiet nan 0x7ff8000000000000.
@wenju-he wenju-he requested a review from Copilot October 15, 2025 08:33
@llvmbot llvmbot added the libclc libclc OpenCL library label Oct 15, 2025
@wenju-he wenju-he requested a review from frasercrmck October 15, 2025 08:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an incorrect NaN mask value for 64-bit floating point numbers in libclc. The value 0x7ff0000000000000 represents positive infinity, not NaN, and is corrected to 0x7ff8000000000000 which represents a quiet NaN.

  • Corrects the NAN_MASK constant for 64-bit floating point from positive infinity to quiet NaN

@wenju-he wenju-he requested a review from arsenm October 15, 2025 08:34
@wenju-he wenju-he changed the title [libclc] Fix double NAN_MASK [libclc] Fix double NAN_MASK in __clc_nan Oct 15, 2025
wenju-he added a commit to wenju-he/llvm that referenced this pull request Oct 15, 2025
…pirv_ocl_nan

NAN_MASK fix is cherry-pick of llvm/llvm-project#163522

Also fix a few build warnings `'__CLC_FUNCTION' macro redefined`.
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Should really just use __builtin_nan

@wenju-he
Copy link
Contributor Author

Should really just use __builtin_nan

done in de867d4
OpenCL spec says The nancode may be placed in the significand of the resulting NaN., so it is probably fine to ignore the input nancode.
IR change:
image
image

@wenju-he wenju-he requested a review from arsenm October 16, 2025 01:45
Copy link
Contributor

@frasercrmck frasercrmck left a comment

Choose a reason for hiding this comment

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

LGTM. I looked at this before but wasn't sure about removing the payload. As you point out it still adheres to the spec so I'm fine with it.

@arsenm
Copy link
Contributor

arsenm commented Oct 18, 2025

Oh right I forgot about the weird string format for __builtin_nan. Technically you could try to find a string to map the value to

@arsenm
Copy link
Contributor

arsenm commented Oct 18, 2025

It's probably better to go back to not using the builtin, failing a way to get the string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libclc libclc OpenCL library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants