Skip to content

Commit

Permalink
Require SSE2 when targetting 32-bit x86
Browse files Browse the repository at this point in the history
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 <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 56d3ad9d23bc130aa9404bfdd1957fe81b3ba498)
  • Loading branch information
davidben authored and justsmth committed Aug 27, 2024
1 parent b0dd757 commit 6fe8dcb
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions crypto/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,18 @@ typedef __uint128_t uint128_t;
#define OPENSSL_SSE2
#endif

// For convenience in testing 64-bit generic code, we allow disabling SSE2
// intrinsics via |OPENSSL_NO_SSE2_FOR_TESTING|. x86_64 always has SSE2
// available, so we would otherwise need to test such code on a non-x86_64
// platform.
#if defined(OPENSSL_X86) && !defined(OPENSSL_NO_ASM) && !defined(OPENSSL_SSE2)
#error \
"x86 assembly requires SSE2. Build with -msse2 (recommended), or disable assembly optimizations with -DOPENSSL_NO_ASM."
#endif

// For convenience in testing the fallback code, we allow disabling SSE2
// intrinsics via |OPENSSL_NO_SSE2_FOR_TESTING|. We require SSE2 on x86 and
// x86_64, so we would otherwise need to test such code on a non-x86 platform.
//
// This does not remove the above requirement for SSE2 support with assembly
// optimizations. It only disables some intrinsics-based optimizations so that
// we can test the fallback code on CI.
#if defined(OPENSSL_SSE2) && defined(OPENSSL_NO_SSE2_FOR_TESTING)
#undef OPENSSL_SSE2
#endif
Expand Down

0 comments on commit 6fe8dcb

Please sign in to comment.