-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[Math] Migrate from Vc to std::experimental::simd
#20746
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: master
Are you sure you want to change the base?
Conversation
Test Results3 754 tests 3 754 ✅ 2h 36m 35s ⏱️ Results for commit e293465. ♻️ This comment has been updated with latest results. |
|
If ROOT is on C++20 and GCC 11 or later in all platforms, you could also consider dropping Vc/VecCore entirely and go straight for std::simd. |
f4ff179 to
b993fb2
Compare
Cool! Yes, that's the plan once we don't have to support |
b993fb2 to
8fa0a2d
Compare
470f198 to
46e5a41
Compare
vc and veccore on march=native buildstd::simd
46e5a41 to
80e05c9
Compare
std::simdstd::simd
80e05c9 to
45c9d2f
Compare
|
Actually, I don't think we should delay this migration to standard C++ features just because of the default compiler on Alma 8. I think we can automatically disable the |
std::simdstd::experimental::simd
|
If you plan to use As for platform support, I don't remember about Windows (although I think it works), but on macOS it works nicely, so I wouldn't disable unless you have specific reasons. It works well to vectorize with ARM Neon. |
|
Thank you very much for keeping this discussion going!
Yes I have to think about that, and it's a good point with the scalar fallback. I don't think we need it actually, but our users might. It depends how much the
You mean |
45c9d2f to
01de1c1
Compare
|
Ok I'm still struggling with the TFormula tests failing on some platforms, because the GCC compiled tests and Cling don't seem to agree what the size of the edit: using the "compatible" tag seems to have worked! And not that this is not a "scalar fallback", as I checked that on my laptop the widths of |
01de1c1 to
12e51c2
Compare
|
@amadio, you have any comments (besides that we can also migrate VecCore, which will happen later)? I would really appreciate your review here! |
… type The GenVector classes are also to be able to accept `std::experimental::simd types` as template parameters, while dropping compatibility with Vc types to keep the overall support burden low. However, if people actually used GenVector with Vc, we can easily fix this for them with some `constexpr` branching in the templated GenVector classes. It's just not worth to do this at this point, as I'm not aware of anyone who used GenVector with Vc.
Always use `std::experimental::simd` as the backend for VecCore if the platform supports it. Otherwise, we drop support for the TMath and TFormula vectorization features that VecCore enables.
We have no test coverage on Linux for these options right now. We don't want to necessarily enable them in the release at this point, but enabling them on the `march=native` build is the perfect opportunity to test the TFormula and TMath features that VecCore enables, in particular with the `std::simd` backend.
Some of the shortcuts represent the same `cmath` functions that are used to implement TMath anyway.
12e51c2 to
8c4657a
Compare
|
The fixed width std::simd performs quite poorly relative to the native version, see https://github.com/root-project/veccore/blob/master/doc/backends.md#c20-simd-backend-using-stdexperimentalsimd I will review the commits and give more detailed feedback later. |
8c4657a to
e293465
Compare
|
Getting rid of Vc would be a huge quality of life improvement. It has been pretty hard to maintain and develop the core infrastructure around the current integration that we have. Please get rid of VecCore, too. |
e293465 to
a293bdb
Compare
Now that we always use the `std::simd` backend of VecCore, we don't need to use `vecCore::math` anymore, because all the functions from `cmath` that VecCore wraps are also implemented for `std::simd`.
a293bdb to
aef208f
Compare
This PR suggests to always use
std::experimental::simdas the backend for VecCore if the platform supports it. Otherwise, we drop support for the TMath and TFormula vectorization features that VecCore enables. The GenVector classes are also changed to be able to acceptstd::experimental::simdtypes as template parameters, while dropping compatibility with Vc types to keep the overall support burden low.In the future, we can also fully replace the VecCore abstraction with
std::experimental::simd.Motivation / what speaks for this PR:
march=native, as many math functions require symbols from Vc, which the interpreter can't look up (Vc is always built statically). This was a fundamental problem with the design. See ROOT-10614.veccore=ONenables without also buildingvc(or getting it viabuiltin_vc=ON)std::experimental::simd)veccorevectorization features also in the CI and and get test coverageNeutral:
veccore=ONdidn't compile on Windows anyway (I checked that with the CI)What you might argue against this PR:
std::simdis not yet available with Apple Clang)std::simd, as this PR suggests to update GenVector to matchstd::simdand compatibility is broken. However, if people actually used GenVector with Vc, we can easily fix this for them with someconstexprbranching in the templated GenVector classes. It's just not worth to do this at this point, as I'm not aware of anyone who used GenVector with Vc.This other PR shows what would happen if
veccore=ONis set for all platforms:std::experimental::simd- show what happens for all platforms withvecccore=ON#20068Closes the following Jira ticket: https://its.cern.ch/jira/browse/ROOT-10614