Skip runtime NEON checks if __ARM_NEON is defined.
When a feature is enabled statically in the build config, the compiler
defines __ARM_NEON and also considers itself free to emit NEON code.
In this case, there is no need to check for NEON support at runtime
since the binary will not work without NEON anyway. Moving the check to
compile time lets us drop unused code.
Chrome has required NEON on Android for nearly five years now. However,
historically there was a bad CPU which broke on some NEON code, but not
others. See https://crbug.com/341598 and https://crbug.com/606629. We
worked around that CPU by parsing /proc/cpuinfo and intentionally
dropping the optimization. This is not a stable situation, however, as
we're hoping the compiler is not good enough at emitting NEON to trigger
this bug.
Since then, the number of affected devices has dropped, and Chrome has
raised the minimum Android requirement to L. The Net.HasBrokenNEON
metric from Chrome is now well in the noise.
This CL stops short of removing the workaround altogether because some
consumers of Cronet are unsure whether they needed this workaround.
Those consumers also build without __ARM_NEON, so gating on that works
out. We'll decide what to do with it pending metrics from them.
Update-Note: Builds with __ARM_NEON (-mfpu=neon) will now drop about
30KiB of dead code, but no longer work (if they even did before) on a
particular buggy CPU. Builds without __ARM_NEON are not affected.
Change-Id: Id8f7bccfb75afe0a1594572ea20c51d275b0a256
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45484
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/cpu-arm-linux.c b/crypto/cpu-arm-linux.c
index c9d771f..962a4a5 100644
--- a/crypto/cpu-arm-linux.c
+++ b/crypto/cpu-arm-linux.c
@@ -175,7 +175,13 @@
hwcap = crypto_get_arm_hwcap_from_cpuinfo(&cpuinfo);
}
- // Clear NEON support if known broken.
+ // Clear NEON support if known broken. Note, if NEON is available statically,
+ // the non-NEON code is dropped and this workaround is a no-op.
+ //
+ // TODO(davidben): The Android NDK now builds with NEON statically available
+ // by default. Cronet still has some consumers that support NEON-less devices
+ // (b/150371744). Get metrics on whether they still see this CPU and, if not,
+ // remove this check entirely.
g_has_broken_neon = crypto_cpuinfo_has_broken_neon(&cpuinfo);
if (g_has_broken_neon) {
hwcap &= ~HWCAP_NEON;
@@ -186,7 +192,10 @@
OPENSSL_armcap_P |= ARMV7_NEON;
// Some ARMv8 Android devices don't expose AT_HWCAP2. Fall back to
- // /proc/cpuinfo. See https://crbug.com/596156.
+ // /proc/cpuinfo. See https://crbug.com/boringssl/46. As of February 2021,
+ // this is now rare (see Chrome's Net.NeedsHWCAP2Workaround metric), but AES
+ // and PMULL extensions are very useful, so we still carry the workaround
+ // for now.
unsigned long hwcap2 = 0;
if (getauxval != NULL) {
hwcap2 = getauxval(AT_HWCAP2);
diff --git a/include/openssl/cpu.h b/include/openssl/cpu.h
index ae55967..22ab7c2 100644
--- a/include/openssl/cpu.h
+++ b/include/openssl/cpu.h
@@ -120,12 +120,7 @@
// CRYPTO_is_NEON_capable returns true if the current CPU has a NEON unit. If
// this is known statically then it returns one immediately.
OPENSSL_INLINE int CRYPTO_is_NEON_capable(void) {
- // Only statically skip the runtime lookup on aarch64. On arm, one CPU is
- // known to have a broken NEON unit which is known to fail with on some
- // hand-written NEON assembly. For now, continue to apply the workaround even
- // when the compiler is instructed to freely emit NEON code. See
- // https://crbug.com/341598 and https://crbug.com/606629.
-#if (defined(__ARM_NEON__) || defined(__ARM_NEON)) && !defined(OPENSSL_ARM)
+#if defined(__ARM_NEON__) || defined(__ARM_NEON)
return 1;
#else
return CRYPTO_is_NEON_capable_at_runtime();
