Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 5a21102

Browse files
davidbenjustsmth
authored andcommittedAug 26, 2024
Require SSE2 when targetting 32-bit x86
Update-Note: Building for 32-bit x86 may require fixing your builds to pass -msse2 to the compiler. This will also speed up the rest of the code in your project. If your project needs to support the Pentium III, please contact BoringSSL maintainers. As far as I know, all our supported 32-bit x86 consumers require SSE2. I think, in the past, I've asserted that our assembly skips SSE2 capability detection. Looking at it again, I don't think that's true. OPENSSL_IA32_SSE2 means to enable runtime detection of SSE2, not compile-time. Additionally, I don't believe we have *ever* tested the non-SSE2 assembly codepaths. Also, now that we want to take the OPENSSL_ia32cap_P accesses out of assembly, those runtime checks are problematic, as we'd need to bifurcafe functions all the way down to bn_mul_words. Unfortunately, the situation with compilers is... complicated. Ideally, everyone would build with the equivalent of -msse2. 32-bit x86 is so register-poor that running without SSE2 statically available seems especially foolish. However, per https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9868, while Clang defaults to enabling SSE2, GCC does not. We once broke gRPC's build, in grpc/grpc#17540, by inadvertently assuming SSE2. In that discussion, gRPC maintainers were okay requiring Pentium 4 as the minimum CPU, but it's unclear if they actually changed their build. That discussion also said GCC 8 assumes SSE2, but I'm not able to reproduce this. LLVM does indeed interpret "i686" as implying SSE2: llvm/llvm-project#61347 rust-lang/rust#82435 However, Debian LLVM does *not*. Debian carries a patch to turn this off! https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/patches/disable-sse2-old-x86.diff?ref_type=heads Meanwhile, Fedora fixed their baseline back in 2018. https://fedoraproject.org/wiki/Changes/Update_i686_architectural_baseline_to_include_SSE2 So let's start by detecting builds that forgot to pass -msse2 and see if we can get them fixed. If this sticks, I'll follow up by unwinding all the SSE2 branches. Bug: 673 Change-Id: I851184b358aaae2926c3e3fe618f3155e71c2f71 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65875 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> (cherry picked from commit 56d3ad9d23bc130aa9404bfdd1957fe81b3ba498)
1 parent bd7dc4d commit 5a21102

File tree

1 file changed

+12
-4
lines changed

1 file changed

+12
-4
lines changed
 

Diff for: ‎crypto/internal.h

+12-4
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,18 @@ typedef __uint128_t uint128_t;
206206
#define OPENSSL_SSE2
207207
#endif
208208

209-
// For convenience in testing 64-bit generic code, we allow disabling SSE2
210-
// intrinsics via |OPENSSL_NO_SSE2_FOR_TESTING|. x86_64 always has SSE2
211-
// available, so we would otherwise need to test such code on a non-x86_64
212-
// platform.
209+
#if defined(OPENSSL_X86) && !defined(OPENSSL_NO_ASM) && !defined(OPENSSL_SSE2)
210+
#error \
211+
"x86 assembly requires SSE2. Build with -msse2 (recommended), or disable assembly optimizations with -DOPENSSL_NO_ASM."
212+
#endif
213+
214+
// For convenience in testing the fallback code, we allow disabling SSE2
215+
// intrinsics via |OPENSSL_NO_SSE2_FOR_TESTING|. We require SSE2 on x86 and
216+
// x86_64, so we would otherwise need to test such code on a non-x86 platform.
217+
//
218+
// This does not remove the above requirement for SSE2 support with assembly
219+
// optimizations. It only disables some intrinsics-based optimizations so that
220+
// we can test the fallback code on CI.
213221
#if defined(OPENSSL_SSE2) && defined(OPENSSL_NO_SSE2_FOR_TESTING)
214222
#undef OPENSSL_SSE2
215223
#endif

0 commit comments

Comments
 (0)
Please sign in to comment.