-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-131032: Fix math.fma(x, y, z) zero sign #131134
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
Conversation
Fix result sign when z is zero. Co-Authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
!buildbot wasi |
🤖 New build scheduled with the buildbot fleet by @skirpichev for commit c95e6f9 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
🤖 New build scheduled with the buildbot fleet by @vstinner for commit c95e6f9 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Nice, test_math pass on WASI buildbots. |
test_fma_zero_result() failed on Android and FreeBSD.
Unrelated failure: test_bigmem failed.
Unrelated failure:
|
Test failing on Android and FreeBSD: # Corner case where rounding the multiplication would
# give the wrong result.
x = float.fromhex('0x1p-500')
y = float.fromhex('0x1p-550')
z = float.fromhex('0x1p-1000')
self.assertIsNegativeZero(math.fma(x-y, x+y, -z))
self.assertIsPositiveZero(math.fma(y-x, x+y, z))
self.assertIsNegativeZero(math.fma(y-x, -(x+y), -z))
self.assertIsPositiveZero(math.fma(x-y, -(x+y), z)) z is not zero in this case. |
Proposed patch affects only case when the last argument ( Netbsd failure also seems related to |
!buildbot wasi |
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
!buildbot FreeBSD |
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
!buildbot Android |
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
It looks this PR does not fix the real issue. It only handles one specific case To handle all cases, we need to implement |
Well, linked issue is a real one. Though, I expected more such cases. (Maybe WASI too?) #131071 is an alternative. BTW, I think that test_fma_zero_result() could be split in that pr as well.
That's true. Only simple workarounds were considered, not implementation of fma() from scratch.
Maybe. But in this case our test suite lacks tests to trigger this. I think that at least failure with musl C stdlib may be related only to arguments with special values. |
@serhiy-storchaka is against this workaround: #131134 (comment). I abandon this approach. |
Fix result sign when z is zero.