591553 - Use NSS_X86_OR_X64 on i386 Mac OS X

Status: RESOLVED FIXED

(NSS :: Build, enhancement, P3)

Description

[Makoto Kato [:m_kato]]

NSS has x86 optimization for byteswap and unalignment access for x86 and x86_64 platform.

http://mxr.mozilla.org/mozilla-central/source/security/nss/lib/freebl/Makefile#83

80 # NSS_X86 means the target is a 32-bits x86 CPU architecture
81 # NSS_X64 means the target is a 64-bits x64 CPU architecture
82 # NSS_X86_OR_X64 means the target is either x86 or x64
83 ifeq (,$(filter-out x386 x86 x86_64,$(CPU_ARCH)))
84         DEFINES += -DNSS_X86_OR_X64
85 ifdef USE_64
86         DEFINES += -DNSS_X64
87 else
88         DEFINES += -DNSS_X86
89 endif
90 endif

But, since i386 uses CPU_ARCH=i386 (TARGET_CPU=i386) on Mac OS X, this platform doesn't get this optimization.

http://mxr.mozilla.org/mozilla/source/security/coreconf/Darwin.mk#49

46 ifndef CPU_ARCH
47 # When cross-compiling, CPU_ARCH should already be defined as the target
48 # architecture, set to powerpc or i386.
49 CPU_ARCH        := $(shell uname -p)
50 endif
51 
52 ifeq (,$(filter-out i%86,$(CPU_ARCH)))
53 ifdef USE_64
54 CC              += -arch x86_64
55 else
56 OS_REL_CFLAGS   = -Di386
57 endif
58 else
59 OS_REL_CFLAGS   = -Dppc
60 endif


We should set CPU_ARCH=x86 into Darwin.mk if CPU_ARCH=i386.  Linux.mk already sets CPU_ARCH=x86 if CPU_ARCH=i386.

Also, about optimization, see bug 431958 and bug 416508.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 3

[Wan-Teh Chang]

Comment on attachment 470259 [details] [diff] [review]
fix

r=wtc.

I verified that no cross-platform or Mac-specific makefile
under mozilla/security compares $(CPU_ARCH) with i386 or i%86,
so this patch won't break any known user of coreconf.

I don't like the use of override, but we can't avoid it
unless we modify Mozilla's makefiles.

I will attach an alternative patch next.

Comment 4

[Wan-Teh Chang]

Attachment: alternative fix

This patch doesn't change coreconf.  It changes the two
NSS makefiles that define NSS_X86_OR_X64, etc.  So it has
a lower risk of breaking other makefiles.

I like m_kato's fix slightly better.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 470259 [details] [diff] [review]
fix

This patch is slightly higher risk than the other, but offers potentially 
greater benefit.  r=nelson

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 491659 [details] [diff] [review]
alternative fix

This is how I would have done it.  I see no reason not to do both. r=nelson

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Bug 591553: Use NSS_X86_OR_X64 on i386 Mac OS X 
Patch contributed by Makoto Kato <[e-mail]>, r=nelson, wtc

Checking in coreconf/Darwin.mk; new revision: 1.28; previous revision: 1.27

Bug 591553: Use NSS_X86_OR_X64 on i386 Mac OS X 
Patch contributed by Wan-Teh Chang <[e-mail]>, r=nelson

Checking in freebl/Makefile; new revision: 1.118; previous revision: 1.117
Checking in jar/config.mk;   new revision: 1.5;   previous revision: 1.4