diff options
| author | Luboš Luňák <l.lunak@collabora.com> | 2022-02-09 16:23:33 +0100 |
|---|---|---|
| committer | Andras Timar <andras.timar@collabora.com> | 2022-02-14 11:00:10 +0100 |
| commit | 70b02b4cbdc424a4abe55f7f175ce6b3ce029862 (patch) | |
| tree | daa5765e3ed3cc6752e4dcea9268b6c4e8d40beb /include/tools | |
| parent | 5057435b7675bae059f1a09139ef6de0e395e4e9 (diff) | |
remove AVX and AVX512 code from Calc
It's been a source of numerous problems since the beginning.
Poor separation of C++ code causing the compiler to emit some generic
code as CPU-specific, compiler optimizations moving CPU-specific
code out of #ifdef to unguarded static initialization, etc.
And it doesn't seem to even particularly improve performance,
on my Ryzen2500U for one full column (1m cells) sumArray() takes
about 1.6ms with AVX, 1.9ms with SSE2 and 4.6ms with generic code.
So SSE2 code is perhaps worth it, especially given that SSE2 is our
baseline requirement on x86_64 everywhere and x86 on Windows,
but AVX+ is nowhere near worth the trouble.
So this code removes all AVX+ code from Calc, and makes SSE2
a hardcoded option on where it's guaranteed. If we raise the baseline
to AVX, the SSE2 code may be replaced by the one removed by this
commit. Generic code is there for other platforms, if other platforms
add CPU-specific code, they should preferably follow the same rules.
This does not necessarily mean that CPU-specific code cannot
be used at all. Some externals use them, for example. It just
needs to be working, maintained, and worth the trouble.
Change-Id: I5ab919930df9d0223db68a94bf84947984d313ac
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129733
Tested-by: Jenkins
Reviewed-by: Eike Rathke <erack@redhat.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129792
Diffstat (limited to 'include/tools')
| -rw-r--r-- | include/tools/cpuid.hxx | 2 | ||||
| -rw-r--r-- | include/tools/simdsupport.hxx | 10 |
2 files changed, 12 insertions, 0 deletions
diff --git a/include/tools/cpuid.hxx b/include/tools/cpuid.hxx index 1be897e84a4e..4f309ff11e96 100644 --- a/include/tools/cpuid.hxx +++ b/include/tools/cpuid.hxx @@ -23,6 +23,8 @@ or inline functions, otherwise their possibly emitted copies compiled with the CPU-specific instructions might be chosen by the linker as the copy to keep. +Also see the note at the top of simdsupport.hxx . + */ namespace cpuid { diff --git a/include/tools/simdsupport.hxx b/include/tools/simdsupport.hxx index bc9227223da0..738b34e072db 100644 --- a/include/tools/simdsupport.hxx +++ b/include/tools/simdsupport.hxx @@ -8,6 +8,16 @@ * */ +// IMPORTANT: Having CPU-specific routines turned out to be a maintenance +// problem, because of various problems such as compilers moving CPU-specific +// code out of #ifdef code into static initialization or our code using C++ +// features that caused the compiler to emit code that used CPU-specific +// instructions (even cpuid.hxx isn't safe, see the comment there). +// The only safe usage is using CPU-specific code that's always available, +// such as SSE2-specific code for x86_64. Do not use for anything else +// unless you really know what you are doing (and you check git history +// to learn from past problems). + // Determine the compiler support for SIMD compiler intrinsics. // This changes from one compiled unit to the other, depending if // the support has been detected and if the compiled unit contains |