Fixed constants::signmask for GCC when using ffast-math
#980
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm fine with the change, as it's not intrusive. Should we try to add validation specific to -ffast-math? |
|
It could be interesting, but it would probably need to adjust the tolerance for errors in the test base, which I'm not familiar with. $ ctest -j3 --output-on-failure
Test project /tmp/tmp.IXhhfXZgB6/xsimd/build
Start 1: test_xsimd
1/1 Test #1: test_xsimd .......................***Failed 0.91 sec
[doctest] doctest version is "2.4.9"
[doctest] run with "--help" for options
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_basic_math.cpp:165:
TEST CASE: [basic math tests]<xsimd::batch<float>>
isfinite
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_basic_math.cpp:43: ERROR: CHECK_FALSE( xsimd::any(xsimd::isfinite(input)) ) is NOT correct!
values: CHECK_FALSE( true )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_basic_math.cpp:165:
TEST CASE: [basic math tests]<xsimd::batch<double>>
isfinite
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_basic_math.cpp:43: ERROR: CHECK_FALSE( xsimd::any(xsimd::isfinite(input)) ) is NOT correct!
values: CHECK_FALSE( true )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_exponential.cpp:182:
TEST CASE: [complex exponential]<xsimd::batch<std::complex<float> >>
huge_exp
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_exponential.cpp:98: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 818, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_exponential.cpp:182:
TEST CASE: [complex exponential]<xsimd::batch<std::complex<double> >>
huge_exp
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_exponential.cpp:98: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 306, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:198:
TEST CASE: [complex trigonometric]<xsimd::batch<std::complex<float> >>
sin
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:67: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 227, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:198:
TEST CASE: [complex trigonometric]<xsimd::batch<std::complex<float> >>
cos
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:83: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 151, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:198:
TEST CASE: [complex trigonometric]<xsimd::batch<std::complex<float> >>
sincos
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:104: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 161, 0 )
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:106: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 161, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:198:
TEST CASE: [complex trigonometric]<xsimd::batch<std::complex<double> >>
sin
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:67: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 91, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:198:
TEST CASE: [complex trigonometric]<xsimd::batch<std::complex<double> >>
cos
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:83: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 91, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:198:
TEST CASE: [complex trigonometric]<xsimd::batch<std::complex<double> >>
sincos
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:104: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 91, 0 )
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_complex_trigonometric.cpp:106: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 91, 0 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_power.cpp:156:
TEST CASE: [power]<xsimd::batch<float>>
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_power.cpp:118: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 39955, 0 )
logged: ipow
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_power.cpp:156:
TEST CASE: [power]<xsimd::batch<double>>
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_power.cpp:118: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 19819, 0 )
logged: ipow
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:195:
TEST CASE: [trigonometric]<xsimd::batch<float>>
trigonometric
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:64: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 89, 0 )
logged: sin
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:80: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 91, 0 )
logged: cos
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:101: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 89, 0 )
logged: sincos(sin)
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:104: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 91, 0 )
logged: sincos(sin)
sincos(cos)
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:120: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 204, 0 )
logged: tan
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:195:
TEST CASE: [trigonometric]<xsimd::batch<double>>
trigonometric
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:64: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 32, 0 )
logged: sin
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:80: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 35, 0 )
logged: cos
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:101: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 32, 0 )
logged: sincos(sin)
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:104: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 35, 0 )
logged: sincos(sin)
sincos(cos)
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_trigonometric.cpp:120: ERROR: CHECK_EQ( diff, 0 ) is NOT correct!
values: CHECK_EQ( 77, 0 )
logged: tan
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_xsimd_api.cpp:747:
TEST CASE: [xsimd api | float types functions]<xsimd::batch<float>>
exp10
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_xsimd_api.cpp:524: ERROR: CHECK_EQ( extract(xsimd::exp10(T(val))), std::pow(value_type(10), val) ) is NOT correct!
values: CHECK_EQ( 100, 100 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_xsimd_api.cpp:747:
TEST CASE: [xsimd api | float types functions]<xsimd::batch<double>>
exp
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_xsimd_api.cpp:516: ERROR: CHECK_EQ( extract(xsimd::exp(T(val))), std::exp(val) ) is NOT correct!
values: CHECK_EQ( 7.38906, 7.38906 )
===============================================================================
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_xsimd_api.cpp:747:
TEST CASE: [xsimd api | float types functions]<xsimd::batch<double>>
expm1
/tmp/tmp.IXhhfXZgB6/xsimd/test/test_xsimd_api.cpp:535: ERROR: CHECK_EQ( extract(xsimd::expm1(T(val))), std::expm1(val) ) is NOT correct!
values: CHECK_EQ( 6.38906, 6.38906 )
===============================================================================
[doctest] test cases: 308 | 296 passed | 12 failed | 0 skipped
[doctest] assertions: 8189 | 8162 passed | 27 failed |
[doctest] Status: FAILURE!
0% tests passed, 1 tests failed out of 1
Total Test time (real) = 0.91 sec
The following tests FAILED:
1 - test_xsimd (Failed)
Errors while running CTest
|
|
Thanks for this fix!
I agree. |
Should I delete it in my PR ? About the tests : it would require a bit more thinking. The tests about NaN / finite and such wouldn't make any sense since fast-math usually disable such kinda logic. For the rest it would require to gather more information of the precision loss. Also : there might be other bugs that I've not encountered. |
|
And you're fine with that? I mean, it does mean that some function are not accurate enough. Let's see if we could relax the accuracy constraint under fast math. |
|
With the precision loss you mean ? So far I'm only using it with fast-math on a hobby project, I don't have a lot of time to put into it. With this fix, the precision of the complex exponential fit my needs, and usually people with high precision constraints won't enable unsafe optimization in the first place. To be more complete it would be interesting to at least have an idea of the precision loss on some setup, to better inform the user. But considering the nature of ffast-math, which can produce really different results based on the fact that the compiler no longer has to give guaranties, it's theoretically not possible to prove it would always behave well. It would only give a hint of what would happen most of the time. |
Yes I think it would make sense. |
JohanMabille
approved these changes
Nov 24, 2023
|
Thanks again! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fix a bug than can occur with
-ffast-math, where the compiler replace -0. values with 0. and therefore breaks thebitofsignfunction as well as several function (exp, sin, cos...) that make use of a bitwise&comparison withconstants::signmaskTo fix the
bitofsignfunction I had to update it and stop usingminuszeroand usesignmaskinstead.minuszerois therefore not used anywhere anymore and could maybe be deleted ?I investigated a bit on how to do the same for clang but I didn't find a solution, and I wasn't able to reproduce the issue with a clang-based aarch64 compiler anyway.
This PR fixes #971