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

fix: Allow building SSE kernels regardless of SSE 4.2 being (in)active #233

Merged
merged 2 commits into from
Dec 19, 2021

Conversation

atteneder
Copy link
Contributor

I tried building KTX-Software for Android x86_64 and for some reason the Android NDK's compiler sets the define __SSE4_2__ even though the SSE version is specified via -msse4.1:

$ /path/to/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ --target=x86_64-none-linux-android24 -msse4.1 -dM -E - < /dev/null | egrep "SSE|AVX" | sort
#define __SSE__ 1
#define __SSE2__ 1
#define __SSE2_MATH__ 1
#define __SSE3__ 1
#define __SSE4_1__ 1
#define __SSE4_2__ 1
#define __SSE_MATH__ 1
#define __SSSE3__ 1

Removing paramter --target=x86_64-none-linux-android24 would solve it, but this is added by the CMake Android toolchain and I'm afraid beyond my control.

This brings me to my question: Does this absolutely have to fail if __SSE4_2__ is set? With the suggested change both basisu and KTX-Software build just fine, although I didn't run it yet.

Thanks for considering!

atteneder added a commit to atteneder/KTX-Software that referenced this pull request May 16, 2021
…roid toolchains mistakenly set SSE4.2 defines (even though only SSE4.1 should be set; see BinomialLLC/basis_universal#233 )
@richgel999
Copy link
Contributor

I think this change would be fine. I'll queue this PR for testing.

@zeux
Copy link
Contributor

zeux commented Jun 7, 2021

Won't the build start failing after this when -msse4.1 is specified on other compilers? I would suggest removing the 4_2 check altogether, as the code doesn't use any 4.2 intrinsics atm.

@atteneder
Copy link
Contributor Author

Won't the build start failing after this when -msse4.1 is specified on other compilers?

I didn't check, but I think you have a point.

I would suggest removing the 4_2 check altogether, as the code doesn't use any 4.2 intrinsics atm.

Agreed, probably the better solution.

@atteneder
Copy link
Contributor Author

Changed it (and tested on Ubuntu x86_64)

@atteneder atteneder changed the title fix: Allow building SSE kernels even when SSE 4.2 is active fix: Allow building SSE kernels regardless of SSE 4.2 being (in)active Jun 21, 2021
@richgel999
Copy link
Contributor

Yea, it doesn't use SSE 4.2. I'll get this in.

@richgel999 richgel999 merged commit 0da4c11 into BinomialLLC:master Dec 19, 2021
@richgel999
Copy link
Contributor

Got it - thank you very much!

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 this pull request may close these issues.

3 participants