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

SSE4.1 support cannot be enabled on machines with AVX #342

Closed
stefantalpalaru opened this issue Jan 31, 2023 · 4 comments · Fixed by #343
Closed

SSE4.1 support cannot be enabled on machines with AVX #342

stefantalpalaru opened this issue Jan 31, 2023 · 4 comments · Fixed by #343

Comments

@stefantalpalaru
Copy link
Contributor

It is due to this check:

#if __AVX__ || __AVX2__ || __AVX512F__
#error Please check your compiler options

Looks like some unfortunate refactoring artefact from old code that supported more than one SIMD type.

@richgel999
Copy link
Contributor

If AVX || AVX2 || AVX512F are defined here then the generated SSE code won't execute on very old (ancient!) CPU's that don't support AVX. That's why these macros are here. I'll double check that this still works, though.

@richgel999
Copy link
Contributor

I've verified that SSE supports works as intended under Windows and Linux. Can you describe what you're doing that's causing these code gen sanity checks to trigger? Your options are probably not the same as the ones used to compile the tool.
I think it's fine to remove them if you're ok with the generated code not working on old CPU's. Perhaps I can put these checks in a guard.

@stefantalpalaru
Copy link
Contributor Author

If __AVX__ || __AVX2__ || __AVX512F__ are defined here

Those are always defined by the compiler, on CPUs that support those features.

I've verified that SSE supports works as intended under Windows and Linux.

Does that CPU support AVX? Try replicating it on a VPS, with a more modern CPU.

Can you describe what you're doing that's causing these code gen sanity checks to trigger?

Trying to enable SSE4.1 by compiling the software on a CPU that supports AVX.

See also: https://bugs.gentoo.org/892727

Your options are probably not the same as the ones used to compile the tool.

cmake -DBUILD_X64=ON -DOPENCL=ON -DSSE=ON -DZSTD=ON

if you're ok with the generated code not working on old CPU's

No such risk. Runtime CPU feature check takes care of that.

The code I'm removing with the related PR, from 2 months ago, is a redundant and buggy compile-time check. No functionality will be lost.

@richgel999
Copy link
Contributor

Odd. I'm going to merge your PR - I don't see any harm in it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants