Review MONO_CPU_X86_SSSE3_COMBINED
#34304
Comments
Dotnet-GitSync-Bot
added
area-Meta
untriaged
|
CC. @EgorBo, @davidwrighton |
|
I'm not convinced that the relationship that is implemented here in Mono is right, but it is interesting. In all practical cases, these may be equivalent, and by combining instruction sets like this, the test matrix is technically reduced. However, we should be sure that the checks in the runtime are equivalent between both Mono and CoreCLR and their associated AOT compilers. One convincing argument that this approach is correct would be if the llvm model also matches mono here. But if it doesn't we should probably change mono to match the Intel/AMD manuals. |
monojenkins
pushed a commit
to monojenkins/mono
that referenced
this issue
Mar 31, 2020
Fixes: dotnet/runtime#34304 @tannergooding @davidwrighton Now it fully matches clang/LLVM behavior, e.g. even `-mattr=+avx2` (or `-mavx2` in clang) doesn't enable `pcmul` and `aes`. However, `-mattr=+sse4.2` (`-msse4.2`) enables `popcnt` (see https://godbolt.org/z/RWAp_D). The intel manual says  So is there a CPU with AVX set but without `popcnt`? The instruction is pretty useful (it is used in BCL) users might forget to define it when they use `mattr=avx` (or `mattr=avx2`). However in C++ world programmers use named groups e.g. `mcpu=haswell` rather than listing these sets via attributes.
akoeplinger
pushed a commit
to mono/mono
that referenced
this issue
Apr 1, 2020
Fixes: dotnet/runtime#34304 @tannergooding @davidwrighton Now it fully matches clang/LLVM behavior, e.g. even `-mattr=+avx2` (or `-mavx2` in clang) doesn't enable `pcmul` and `aes`. However, `-mattr=+sse4.2` (`-msse4.2`) enables `popcnt` (see https://godbolt.org/z/RWAp_D). The intel manual says  So is there a CPU with AVX set but without `popcnt`? The instruction is pretty useful (it is used in BCL) users might forget to define it when they use `mattr=avx` (or `mattr=avx2`). However in C++ world programmers use named groups e.g. `mcpu=haswell` rather than listing these sets via attributes. Co-authored-by: EgorBo <EgorBo@users.noreply.github.com>
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
MONO_CPU_X86_SSSE3_COMBINEDis currently defined asMONO_CPU_X86_SSE3_COMBINED | MONO_CPU_X86_PCLMUL | MONO_CPU_X86_AES | MONO_CPU_X86_SSSE3,(runtime/src/mono/mono/mini/mini.h
Line 2832 in 335ffb0
However, this doesn't match the hierarchy exposed by
System.Runtime.Intrinsicsnor does it match the CPUID checks specified by the Intel or AMD architecture manuals:From
Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 1: Basic Architecture, October 2019The text was updated successfully, but these errors were encountered: