Skip to content

Conversation

@N5N3
Copy link
Member

@N5N3 N5N3 commented Dec 23, 2021

This PR aims to fix runtime fma on windows by adding julia_fma(f) using Base.fma_emulated as the reference.
On other systems, julia_fma(f) call use fma(f) directly.
I use Base.fma_float to test julia_fma(f)'s precison. And it passed locally.
Since it only affects const propagation, I believe perfomance is not important.
We can remove this code if fma on windows get fixed in the future.
Close #43386.

@N5N3 N5N3 closed this Dec 23, 2021
@N5N3 N5N3 reopened this Dec 24, 2021
@oscardssmith oscardssmith added maths Mathematical functions system:windows Affects only Windows labels Dec 24, 2021
@oscardssmith
Copy link
Member

having written the Julia version of this, congrats on having the fortitude it takes to rewrite this! This looks like the best solution to windows being bad for now.

@N5N3
Copy link
Member Author

N5N3 commented Dec 25, 2021

@oscardssmith Linux32's runtime fma seems also broken.
Is it OK to skip the test on Linux32 in this PR? (I have no linux machine.)

@oscardssmith
Copy link
Member

32 bit linux is passing those tests on master, so they should continue to be tested and pass.

@N5N3
Copy link
Member Author

N5N3 commented Dec 25, 2021

master doesn't test runtime fma (call Base.fma_float(a, b, c) directly).
I mean keep test fma and Base.fma_emulated, and skip Base.fma_float only.

@oscardssmith
Copy link
Member

that should be OK.

@N5N3 N5N3 force-pushed the windowsfma branch 2 times, most recently from c74e4a2 to 063b89c Compare December 25, 2021 06:53
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

LGTM. What is wrong with linux32?

@N5N3
Copy link
Member Author

N5N3 commented Jan 7, 2022

I'm not sure, runtime fma test passed locally using official 32bit 1.7.1 on wsl2.
But CI test kept failing. (So I skip it on Linux32).
The current error should be unrelated.

N5N3 added 3 commits January 15, 2022 14:35
1. add `volatile` to fix Win32
2. fix `issubnormal` on Win32
@N5N3
Copy link
Member Author

N5N3 commented Jan 15, 2022

All test passed after rebasing @vtjnash

@oscardssmith oscardssmith merged commit 49c667d into JuliaLang:master Jan 15, 2022
@N5N3 N5N3 deleted the windowsfma branch January 15, 2022 15:38
N5N3 added a commit to N5N3/julia that referenced this pull request Jan 24, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maths Mathematical functions system:windows Affects only Windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FMA regression on windows on master (CPU:9600KF)

4 participants