Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

@parjong
Copy link

@parjong parjong commented Nov 29, 2016

This commit disables FEATURE_AVX_SUPPORT for x86/Linux to fix #8331.

This commit disables FEATURE_AVX_SUPPORT for x86/Linux to fix #8331.
@parjong parjong changed the title Disable FEATURE_AVX_SUPPORT for x86/Linux [x86/Linux] Disable FEATURE_AVX_SUPPORT Nov 29, 2016
@parjong
Copy link
Author

parjong commented Nov 29, 2016

\CC @seanshpark

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)
Copy link
Member

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

Copy link
Member

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.

Copy link
Author

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.

@seanshpark
Copy link

Comment from @BruceForstall , "and FEATURE_SIMD as well", is it ok with "SIMD"?

@parjong
Copy link
Author

parjong commented Nov 29, 2016

@seanshpark Actually, I didn't find any issue with FEATURE_SIMD, yet. But, it would be better to disable both of them.

@BruceForstall
Copy link

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 if but instead use:

if (CLR_CMAKE_TARGET_ARCH_AMD64 OR (CLR_CMAKE_TARGET_ARCH_I386 AND NOT CLR_CMAKE_PLATFORM_UNIX))

?

@BruceForstall
Copy link

LGTM

@janvorli janvorli merged commit 9883c46 into dotnet:master Nov 30, 2016
@parjong parjong deleted the fix/issue_8331 branch November 30, 2016 23:13
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RyuJIT/x86: encoder uses size_t to accumulate encoding bits, but that's not big enough

6 participants