From 332047c429ea684f09c68ae3ddd0ea988a713b27 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 14 Jan 2020 16:20:39 +0000 Subject: [PATCH] Bug 1608327 - Fix freebl arm NEON code use on tier3 platforms. r=jcj Despite the code having runtime detection of NEON and crypto extensions, the optimized code using those instructions is disabled at build time on platforms where the compiler doesn't enable NEON by default of with the flags it's given for the caller code. In the case of gcm, this goes as far as causing a build error. What is needed is for the optimized code to be enabled in every case, letting the caller code choose whether to use that code based on the existing runtime checks. But this can't be simply done either, because those optimized parts of the code need to be built with NEON enabled, unconditionally, but that is not compatible with platforms using the softfloat ABI. For those, we need to use the softfp ABI, which is compatible. However, the softfp ABI is not compatible with the hardfp ABI, so we also can't unconditionally use the softfp ABI, so we do so only when the compiler targets the softfloat ABI, which confusingly enough is advertized via the `__SOFTFP__` define. Differential Revision: https://phabricator.services.mozilla.com/D59451 --HG-- extra : moz-landing-system : lando --- lib/freebl/Makefile | 8 ++++++-- lib/freebl/aes-armv8.c | 2 +- lib/freebl/freebl.gyp | 17 +++++++++++++---- lib/freebl/gcm-arm32-neon.c | 4 ++-- lib/freebl/gcm.c | 7 ++----- lib/freebl/rijndael.c | 3 +-- 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/freebl/Makefile b/lib/freebl/Makefile index 0156a0842e..d94cb0d480 100644 --- a/lib/freebl/Makefile +++ b/lib/freebl/Makefile @@ -765,8 +765,12 @@ $(OBJDIR)/$(PROG_PREFIX)intel-gcm-wrap$(OBJ_SUFFIX): CFLAGS += -mssse3 endif ifeq ($(CPU_ARCH),arm) -$(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a -mfpu=crypto-neon-fp-armv8 -$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -mfpu=neon +# When the compiler uses the softfloat ABI, we want to use the compatible softfp ABI when +# enabling NEON for these objects. +# Confusingly, __SOFTFP__ is the name of the define for the softfloat ABI, not for the softfp ABI. +USES_SOFTFLOAT_ABI := $(shell $(CC) -o - -E -dM - $(CFLAGS) < /dev/null | grep __SOFTFP__ > /dev/null && echo 1) +$(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a -mfpu=crypto-neon-fp-armv8$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp) +$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp) endif ifeq ($(CPU_ARCH),aarch64) $(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a+crypto diff --git a/lib/freebl/aes-armv8.c b/lib/freebl/aes-armv8.c index 8213272f53..7be39ede89 100644 --- a/lib/freebl/aes-armv8.c +++ b/lib/freebl/aes-armv8.c @@ -8,7 +8,7 @@ #if ((defined(__clang__) || \ (defined(__GNUC__) && defined(__GNUC_MINOR__) && \ (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 8)))) && \ - (defined(__ARM_NEON) || defined(__ARM_NEON__))) + defined(IS_LITTLE_ENDIAN)) #ifndef __ARM_FEATURE_CRYPTO #error "Compiler option is invalid" diff --git a/lib/freebl/freebl.gyp b/lib/freebl/freebl.gyp index 0b05a2b246..d01531ad19 100644 --- a/lib/freebl/freebl.gyp +++ b/lib/freebl/freebl.gyp @@ -158,10 +158,12 @@ '<(DEPTH)/exports.gyp:nss_exports' ], 'cflags': [ - '-mfpu=neon' + '-mfpu=neon', + '<@(softfp_cflags)', ], 'cflags_mozilla': [ - '-mfpu=neon' + '-mfpu=neon', + '<@(softfp_cflags)', ] }, { @@ -211,11 +213,13 @@ [ 'target_arch=="arm"', { 'cflags': [ '-march=armv8-a', - '-mfpu=crypto-neon-fp-armv8' + '-mfpu=crypto-neon-fp-armv8', + '<@(softfp_cflags)', ], 'cflags_mozilla': [ '-march=armv8-a', - '-mfpu=crypto-neon-fp-armv8' + '-mfpu=crypto-neon-fp-armv8', + '<@(softfp_cflags)', ], }, 'target_arch=="arm64" or target_arch=="aarch64"', { 'cflags': [ @@ -567,6 +571,11 @@ }, { 'have_int128_support%': 0, }], + [ 'target_arch=="arm"', { + # When the compiler uses the softfloat ABI, we want to use the compatible softfp ABI when enabling NEON for these objects. + # Confusingly, __SOFTFP__ is the name of the define for the softfloat ABI, not for the softfp ABI. + 'softfp_cflags': ' /dev/null && echo -mfloat-abi=softfp || true)', + }], ], } } diff --git a/lib/freebl/gcm-arm32-neon.c b/lib/freebl/gcm-arm32-neon.c index 97eb82ec62..be04247701 100644 --- a/lib/freebl/gcm-arm32-neon.c +++ b/lib/freebl/gcm-arm32-neon.c @@ -11,7 +11,7 @@ #include "secerr.h" #include "prtypes.h" -#if defined(__ARM_NEON__) || defined(__ARM_NEON) +#if defined(IS_LITTLE_ENDIAN) #include @@ -199,4 +199,4 @@ gcm_HashZeroX_hw(gcmHashContext *ghash) return SECSuccess; } -#endif /* __ARM_NEON__ || __ARM_NEON */ +#endif /* IS_LITTLE_ENDIAN */ diff --git a/lib/freebl/gcm.c b/lib/freebl/gcm.c index 080e641ea4..2a42f74c0e 100644 --- a/lib/freebl/gcm.c +++ b/lib/freebl/gcm.c @@ -21,11 +21,8 @@ #if defined(__aarch64__) && defined(IS_LITTLE_ENDIAN) && \ (defined(__clang__) || defined(__GNUC__) && __GNUC__ > 6) #define USE_ARM_GCM -#elif defined(__arm__) && defined(IS_LITTLE_ENDIAN) && \ - (defined(__ARM_NEON__) || defined(__ARM_NEON)) -/* We don't test on big endian platform, so disable this on big endian. - * Also, we don't check whether compiler support NEON well, so this uses - * that compiler uses -mfpu=neon only. */ +#elif defined(__arm__) && defined(IS_LITTLE_ENDIAN) +/* We don't test on big endian platform, so disable this on big endian. */ #define USE_ARM_GCM #endif diff --git a/lib/freebl/rijndael.c b/lib/freebl/rijndael.c index 40364fce0f..2e8bab87ff 100644 --- a/lib/freebl/rijndael.c +++ b/lib/freebl/rijndael.c @@ -20,8 +20,7 @@ #include "gcm.h" #include "mpi.h" -#if (!defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64)) || \ - (defined(__arm__) && !defined(__ARM_NEON) && !defined(__ARM_NEON__)) +#if !defined(IS_LITTLE_ENDIAN) && !defined(NSS_X86_OR_X64) // not test yet on big endian platform of arm #undef USE_HW_AES #endif