-
Notifications
You must be signed in to change notification settings - Fork 221
Replaced test values for hexfloat and printf to avoid ambiguity #2425
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
base: main
Are you sure you want to change the base?
Conversation
bashbaug
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We pass the test with these changes FWIW, but we didn't have a problem with the prior test either, so I'm not sure how much value this brings.
Can you please explain how choosing different floating-point values avoids the ambiguity? If I'm reading the StackOverflow link from the issue correctly every finite and non-zero floating-point number has four valid hex float representations, so choosing different floating-point values won't necessarily help.
@bashbaug this is correct, theoretically, multiple hexadecimal representations exist for the same floating-point value (like |
|
@alycm filed the related issue and may be able to provide additional guideance. |
|
Discussed in the September 9th teleconference. We don't think this PR will fix the fundamental problem, but somebody from Imagination (ping @paulfradgley) will confirm. |
|
Closing without merging. This doesn't fix the reported issue, so we will need an alternative solution. |
|
@bashbaug Could we reopen this PR? Based on the earlier discussion, I was expecting feedback from other vendors and waited accordingly. I’ve now added a new commit with a minimal and non-intrusive alternative proposal. Could we revisit this solution? |
|
Reopened as per request above. |
|
I've introduced an alternative approach that performs a fallback test when enabled in the test configuration and the regular test fails. This solution verifies |
Fixes #1335 according to the issue description.
@alycm replaced with straightforward values in terms of hex float representation