1646594 - AVX2 is not enabled in x86_64 Linux with make 4.3

Status: RESOLVED FIXED

(NSS :: Build, enhancement, P1)

Description

[Danh]

Attachment: 0001-enable-AVX2-if-applicable-on-x86_64.patch

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

export NSPR_INCLUDE_DIR=/usr/include/nspr
export NSPR_LIB_DIR=/usr/lib
export CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 -g -O2"
make USE_64=1

Actual results:

nss/lib/freebl/verified/Hacl_Chacha20Poly1305_256.c, and other source files relied on avx2 are not compiled

Expected results:

nss/lib/freebl/verified/Hacl_Chacha20Poly1305_256.c and other source files relied on avx2 are compiled.

nss test suite should be passed in x86_64 hardware regardless of avx2 support.

I have this patch that works for NSS 3.53, but the test suite is failing in NSS 3.53.1, I'm running NSS 3.53.1 testsuide without this patch.

Comment 1

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 9157527 [details] [diff] [review]
0001-enable-AVX2-if-applicable-on-x86_64.patch

Ben, can you review?

Comment 2

[Danh]

TEST_P(TlsExtensionTest13, HrrThenRemoveSignatureAlgorithms)

This one failed intermittently on SandyBrigde,
I enable the patch again and it doesn't fail now.

Comment 3

[Benjamin Beurdouche [:beurdouche]]

Hi Danh, can you please push this on Phabricator and assign [bbeurdouche] for review there? Thanks

Comment 4

[Danh]

Attachment: enable AVX2 if applicable on x86_64 make 4.3

https://bugzilla.mozilla.org/show_bug.cgi?id=1646594

System: Linux x86_64
Build System: GNU make 4.3

Comment 5

[Danh]

Hi Ben,

I uploaded to https://phabricator.services.mozilla.com/D81110

Comment 6

[Benjamin Beurdouche [:beurdouche]]

Thanks for the patch, it passes CI so we'll land it shortly... : )

Comment 7

[J.C. Jones [:jcj] (he/him)]

https://hg.mozilla.org/projects/nss/rev/b579895aceb0d36fc586e9031d8c5d7a4111201a

Comment 8

[Michael Stahl]

(In reply to J.C. Jones [:jcj] (he/him) [increased latency due to COVID-19] from comment #7)

https://hg.mozilla.org/projects/nss/rev/b579895aceb0d36fc586e9031d8c5d7a4111201a

this broke the Android ARM build:

mpi/mpcpucache.c:91:15: error: invalid output constraint '=a' in asm
: "=a"(*eax),
^

the compile command contains -Di386 which originates from:

makefile (from '../../coreconf/Linux.mk', line 74)

OS_REL_CFLAGS = -Di386

this is because of the include Linux.mk added in the commit; at the time in arch.mk not all variables that affect Linux.mk have been initialized yet and so it sets x86 specific variables, which are not all overwritten when Linux.mk is included a second time.

the easiest fix for me would be to add ifneq ($(OS_TARGET),Android) around that, but it's possible that this breaks other non-Android configs too.

so Android.mk includes Linux.mk which sets CC and includes UNIX.mk which includes Werror.mk which sets GCC_VERSION...

i'm thinking the whole ifndef NSS_DISABLE_AVX2 block should be moved from arch.mk to a different file that is read after config.mk include coreconf/$(OS_TARGET).mk, but i'm not sure where to move it to.

Comment 9

[J.C. Jones [:jcj] (he/him)]

Thanks, Michael. Since there's been a release inbetween, I opened bug 1659727 for the regression.