Closed Bug 1646594 Opened 1 year ago Closed 1 year ago

AVX2 is not enabled in x86_64 Linux with make 4.3

Categories

(NSS :: Build, enhancement, P1)

3.53
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: congdanhqx, Assigned: congdanhqx)

References

Details

Attachments

(2 files)

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 on attachment 9157527 [details] [diff] [review]
0001-enable-AVX2-if-applicable-on-x86_64.patch

Ben, can you review?
Attachment #9157527 - Flags: review?(bbeurdouche)

TEST_P(TlsExtensionTest13, HrrThenRemoveSignatureAlgorithms)

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

Assignee: nobody → sgn.danh
Severity: -- → N/A
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1

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

Flags: needinfo?(sgn.danh)

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

System: Linux x86_64
Build System: GNU make 4.3

Flags: needinfo?(sgn.danh)

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

Attachment #9157527 - Flags: review?(bbeurdouche) → review+
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 3.55

(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.

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

You need to log in before you can comment on or make changes to this bug.