Skip to content
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

[libwebp] Fix SIMD patch quirks #32985

Merged
merged 5 commits into from
Aug 8, 2023
Merged

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Aug 5, 2023

Reverts #21644.
Fixes #14568.
Fixes #32940.

@dg0yt dg0yt mentioned this pull request Aug 5, 2023
@LilyWangLL LilyWangLL added the category:port-bug The issue is with a library, which is something the port should already support label Aug 7, 2023
LilyWangLL
LilyWangLL previously approved these changes Aug 7, 2023
@LilyWangLL LilyWangLL added info:reviewed Pull Request changes follow basic guidelines and removed info:reviewed Pull Request changes follow basic guidelines labels Aug 7, 2023
@dg0yt dg0yt marked this pull request as ready for review August 7, 2023 06:49
Comment on lines +9 to +10
- set(SIMD_ENABLE_FLAGS "/arch:AVX;/arch:SSE2;;;;")
+ set(SIMD_ENABLE_FLAGS ";/arch:SSE2;;;;") # /arch:AVX is too much for SSE4
Copy link
Contributor Author

@dg0yt dg0yt Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is based on #21644 / #14568.

IMO this is a pessimistic build-time fix for generating invalid code for non-avx sse4.1 CPUs (#14568). AFAIU the actual implementation is chosen at runtime. So the proper fix would be to choose the the sse4.1 code only if the CPU supports AVX if the compiler is MSVC. Something around these lines:
https://github.com/webmproject/libwebp/blob/dd7364c3cefe0f5c0b3c18c3b1887d353f90fc1f/src/dsp/cpu.c#L158-L167
But I don't want to touch that now. This cmake code path is not used for modern versions of MSVC.

@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Aug 8, 2023
@JavierMatosD JavierMatosD merged commit 3a30230 into microsoft:master Aug 8, 2023
15 checks passed
@dg0yt dg0yt deleted the libwebp branch August 8, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libwebp] Build error [libwebp] Do not enable SSE4 support with /arch:AVX
3 participants