-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[x86/Linux] Disable FEATURE_AVX_SUPPORT #8335
Conversation
This commit disables FEATURE_AVX_SUPPORT for x86/Linux to fix #8331.
|
\CC @seanshpark |
src/jit/CMakeLists.txt
Outdated
| if (CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_I386) | ||
| add_definitions(-DFEATURE_SIMD) | ||
| add_definitions(-DFEATURE_AVX_SUPPORT) | ||
| if (!CLR_CMAKE_PLATFORM_UNIX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to disable it for AMD64 UNIX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, could you please use "NOT" instead of "!"? We do it this way in all the CMakeFiles.txt and in fact, I have not found the "!" documented anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that was my mistake. Thanks you for comment. I'll revise PR per feedback.
|
Comment from @BruceForstall , "and FEATURE_SIMD as well", is it ok with "SIMD"? |
|
@seanshpark Actually, I didn't find any issue with FEATURE_SIMD, yet. But, it would be better to disable both of them. |
|
I agree it makes sense to disable them both and then, when you're ready to work on Linux/x86 SIMD, re-enable them both. Couldn't you then not have nested ? |
|
LGTM |
* Disable FEATURE_AVX_SUPPORT for x86/Linux This commit disables FEATURE_AVX_SUPPORT for x86/Linux to fix dotnet/coreclr#8331. * Disable FEATURE_AVX_SUPPORT only for x86/Linux * Disable FEATURE_SIMD for x86/Linux * Simplify nested if in CMakeList.txt Commit migrated from dotnet/coreclr@9883c46
This commit disables FEATURE_AVX_SUPPORT for x86/Linux to fix #8331.