Improve DES and SHA512 for x86_64 platform

RESOLVED FIXED in 3.12.4

Status

defect
P1
blocker
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: m_kato, Assigned: julien.pierre)

Tracking

trunk
3.12.4
x86
All
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: FIPS _X86_)

Attachments

(6 attachments, 3 obsolete attachments)

Some chipper code isn't be optimized for x86_64 platform.

After applying an attached patch, DES improves 2-3% and SHA512 improves 5% on x86_64 Linux and Windows.
Attached patch?  I see no attached patch.

On what chips, specifically, does this patch improve performance?
Present code was optimized for AMD Opteron.  
Code optimized for Opteron is known to not be optimized for certain 
Intel CPUs.  We must choose our battles.
Posted patch a patch for x86_64 (obsolete) — Splinter Review
(In reply to comment #1)
> Attached patch?  I see no attached patch.
> 
> On what chips, specifically, does this patch improve performance?
> Present code was optimized for AMD Opteron.  
> Code optimized for Opteron is known to not be optimized for certain 
> Intel CPUs.  We must choose our battles.
> 

I tested on AMD Turion 64 and Core 2 Duo.  Byte swapping should use inline assember on x64, and x64 does not need alignment access.
Attachment #319104 - Flags: review?(nelson)
Severity: normal → enhancement
Comment on attachment 319104 [details] [diff] [review]
a patch for x86_64

Makoto,  Here are some initial preliminary review comments.

Your patch makes the following change in many places:

>-#if defined (_X86_)
>+#if defined (_X86_) || defined(__x86_64__) || defined(__x86_64)

If I recall correctly, the symbol _X86_ (notice the upper case X) 
is supposed to be defined on all x86 family architectures.
It is defined in the Makefile (or should be), not by any compiler.  

Rather than changing all the lines in many files that test for _X86_ 
to also test for other values, the _X86_ symbol should be defined 
in the makefile for all CPUs that need the _X86_ code.

Have you found any places in the code where _X86_ code is not 
appropriate for x86_64 CPUs also?  
If so, please tell us about them in a comment to this bug.
Otherwise, it would be preferable for the patch to define _X86_ 
for those CPUs, and NOT change all the tests for _X86_ to also
test for other x86b symbols. 

I will do a more complete review of this patch in a few days.
(In reply to comment #4)

On x86_64 gcc 4.2.3 (I use Ubuntu 8.04 x64), _X86_ is not defined on it.

This is sample test.

#include <stdio.h>

int main()
{
#if defined(_X86_)
        printf("x86\n");
#elif defined(__x86_64__)
        printf("x64\n");
#endif
        return 0;
}

makoto@localhost:~/Development$ gcc test.c
makoto@localhost:~/Development$ ./a.out
x64


Also, I don't check all _X86_ define for NSS.  I will check it.
Makoto, I repeat:
> It is defined in the Makefile (or should be), not by any compiler.  
(In reply to comment #6)
> Makoto, I repeat:
> > It is defined in the Makefile (or should be), not by any compiler.  

Is is defined by GCC.  (On MSVC, it is not defined on AMD64 cl.exe.  I don't know about Intel compiler.) This should be used by optimization such as inline assembler.

And about alignment access or not, maybe, we should use new define (ex. NSS_NO_ALIGNMENT_ACCESS?).
(In reply to comment #8)
> Please observe these definitions of _X86_ in Makefiles:
> http://mxr.mozilla.org/security/search?string=%3D.*_X86_&regexp=on&case=on&find=%2Fsecurity%2F&tree=security
> 

Oops!.  I misunderstand this.  I will create new smart patch for this.
Comment on attachment 319104 [details] [diff] [review]
a patch for x86_64

Because of comment 9, I believe this patch (and its review request) are now obsolete.
Attachment #319104 - Attachment is obsolete: true
Attachment #319104 - Flags: review?(nelson)
Attachment #322055 - Flags: review?(nelson)
Blocks: FIPS2008
Priority: -- → P4
Whiteboard: FIPS
Target Milestone: --- → 3.12.7
If this bug is completed by Nov17 2008 it will be included in the FIPS2008 validation otherwise it will be dropped for a later release.
Attachment #322055 - Flags: review?(nelson) → review+
Comment on attachment 322055 [details] [diff] [review]
freebl patch for x86 and x86_64 optimization v2

r=nelson
Comment on attachment 322055 [details] [diff] [review]
freebl patch for x86 and x86_64 optimization v2

In mozilla/security/nss/lib/freebl/Makefile, we have

>-ifeq ($(CPU_ARCH),x86)
>+ifeq (,$(filter-out x86 x86_64,$(CPU_ARCH)))
> ifneq (,$(filter-out WIN%,$(OS_TARGET)))
> 	OS_REL_CFLAGS += -D_X86_
> endif
> endif

This change is wrong.  _X86_ has always meant that we're compiling
for the 32-bit x86 instruction set.  When $(CPU_ARCH) is x86_64, it
means we're compiling for the 64-bit x86_64 instruction set.  It
doesn't mean the build machine's processor is x86_64.  See this
snippet from mozilla/security/coreconf/Linux.mk:

  ifeq ($(OS_TEST),x86_64)
  ifeq ($(USE_64),1)
          OS_REL_CFLAGS   = -DLINUX1_2 -D_XOPEN_SOURCE
          CPU_ARCH        = x86_64
  else
          OS_REL_CFLAGS   = -DLINUX1_2 -Di386 -D_XOPEN_SOURCE
          CPU_ARCH        = x86
          ARCHFLAG        = -m32
  endif
  else

It shows that if the build machine's processor ($(OS_TEST)) is
x86_64 but USE_64 is not set, we set CPU_ARCH to x86.

I think this is why you need this change in
mozilla/security/nss/lib/freebl/sha512.c:

>-#if defined(_X86_) || defined(SHA_NO_LONG_LONG)
>+#if (defined(_X86_) && !defined(__x86_64__)) || defined(SHA_NO_LONG_LONG)

(defined(_X86_) && !defined(__x86_64__)) looks wrong.

Does Visual C++ define _X86_ (you need to include <windows.h>) when
you compile for 64-bit Windows?
Checking in Makefile; new revision: 1.95; previous revision: 1.94
Checking in des.c;    new revision: 1.6;  previous revision: 1.5
Checking in sha512.c; new revision: 1.13; previous revision: 1.12
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: 3.12.7 → 3.12.3
Nelson, are you sure you want this change (comment 4)?

-# des.c wants _X86_ defined for intel CPUs.  
+# some codes want _X86_ defined for intel CPUs.  
 # coreconf does this for windows, but not for Linux, FreeBSD, etc.
-ifeq ($(CPU_ARCH),x86)
+ifeq (,$(filter-out x86 x86_64,$(CPU_ARCH)))
 ifneq (,$(filter-out WIN%,$(OS_TARGET)))
 	OS_REL_CFLAGS += -D_X86_
 endif
 endif

This change enables all the _X86_ code for x86-64 on non-Windows
platforms.  It has two problems:
1. Its implications are larger than the patch shows. alg2268.c,
desblapi.c, md5.c, rijndael.c, and sha_fast.h also test the _X86_
macro.  All of that code needs to be reviewed.
2. It creates a discrepancy between Windows and non-Windows platforms.
MSVC does *not* define _X86_ when targeting x86-64.  The new,
broadened meaning of _X86_ no longer matches its original meaning
in MSVC.

It is less confusing to define a new macro that covers both x86 and
x86-64.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Wan-Teh, I wrote much of the code in the files you cited, and I believe I 
understand how they all use the _X86_ macro.  I believe they use it to 
mean/imply the following 3 properties about the CPU
1) allows unaligned loads and stores 
2) is little-endian
3) supports the bswap instruction (by various names)

All those things are true for both 32-bit and 64-bit x86-family CPUs.

The file sha512.c also used it to mean/imply that 64-bit operations were
inefficient and that the CPU is register starved, and so code that 
relies on 64-bit operations and or lots of registers should be avoided.
Makoto clearly recognized that in his patch for sha512.c, but even if he
had not, I believe the results would merely have been suboptimal, not 
incorrect.  

I observe that Makoto's patches have not caused any failures in the freebl 
blapitest tests, which test the algorithms for correctness.  I'll say more 
in email.
Thanks.  So the code is correct.  Then the only issue is the
discrepancy between the meanings of _X86_ on Windows and
non-Windows platforms.

On Windows, <windows.h> defines _X86_ and _AMD64_ as follows
(_M_IX86 and _M_AMD64 are compiler predefined macros):

  #if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && defined(_M_IX86)
  #define _X86_
  #endif

  #if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && defined(_M_AMD64)
  #define _AMD64_
  #endif

A few Windows system headers check _X86_, so we cannot define _X86_
for x86-64 systems on Windows.  (That'd be equivalent to defining
__i386__ for x86-64 systems on Linux.)

So, _X86_ now means "x86" on Windows, and "x86 or x86-64" on
non-Windows platforms.  I find this very confusing.  Here is a patch
that adds a comment to lib/freebl/Makefile to document the meaning of
_X86_.
Attachment #349242 - Flags: review?(nelson)
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Comment on attachment 349242 [details] [diff] [review]
Add a comment to explain the two meanings of _X86_

Your comment is actually incorrect.

On Solaris, with the Studio 12 compiler, the _X86_ symbol is defined only in the 32-bit case, and not in the 64-bit case, ie. just like Windows.

I agree with you that this is all very confusing. Perhaps we should not rely on _X86_ if some platforms are defining it even in the 64-bit case. Maybe we should use some other symbol of our own that means "32-bit x86", which would not be subject to ambiguity.
Attachment #349242 - Flags: superreview-
mozilla/security/coreconf/{SunOS5.10_i86pc.mk,SunOS5.11_i86pc.mk}
should set CPU_ARCH to x86_64.  Did I miss something?  Were you
on Solaris 8 or 9 by any chance?

If CPU_ARCH is x86_64, it should cause the following code in
lib/freebl/Makefile to define _X86_:

 ifeq (,$(filter-out x86 x86_64,$(CPU_ARCH)))
 ifneq (,$(filter-out WIN%,$(OS_TARGET)))
 	OS_REL_CFLAGS += -D_X86_
 endif
 endif
Considering all the confusion this has caused to my colleagues, I think
that it would be advisable to change this code to go back to a state where
_X86_ means 32-bit x86 only.  I apologize to Makoto Kato for making him 
change his patch in a way that must now be undone.  :-/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #349242 - Flags: review?(nelson)
Comment on attachment 349242 [details] [diff] [review]
Add a comment to explain the two meanings of _X86_

Rather than review this patch, I'll work on making the comment unnecessary.
Wan-Teh,

Re: comment 20, no, I was on Solaris 10, and I have confirmed that _X86_ is not defined when building 64-bit on s10, even in freebl .

The Makefile section that you quoted modifies OS_REL_CFLAGS . But that variable is not used by coreconf on Solaris . So that section has no effect on Solaris.
Thanks for the explanation.  I didn't know that OS_REL_CFLAGS
is not used by coreconf on Solaris:
http://mxr.mozilla.org/security/search?string=OS_REL_CFLAGS

I think that's a bug.  I believe we intended to define _X86_
for all OSes running on x86.  We should use DEFINES instead.
Wan-Teh,

I am not sure what the original intent was anymore. But clearly we were not consistent about what was done. Personally, I prefer to see _X86_ used only to mean "32-bit x86". I don't think it helps to have _X86_ defined on x86_64 platforms, since we can't directly link or invoke 32-bit x86 assembler code, and even certain C constructs that use 32-bit variables would be better off with 64-bit variables.
OS_REL_CFLAGS appears to be an internal coreconf macro. It is only used on some platforms. It was never used anywhere in all of security/nss before attachment 322055 [details] [diff] [review] . We should definitely stop using it. DEFINES would be the correct macro for this usage.

However, the question remains about whether we want to define _X86_ for both 32-bit and 64-bit cases. Fortunately, this macro is only used in freebl and mpi as far as NSS is concerned.

But there are many source files that test it :

alg2268.c
des.c
desblapi.c
md5.c
rijndael.c
sha512.c
sha_fast.h
mpcpucache.c

Unfortunately, we don't run regular performance tests for any of these algorithms. And I am not aware that we have made any comparisons before or after the patch for this bug was checked in.

The current situation is that we have _X86_ defined on Linux x64, and not defined on Windows x64 or Solaris x64. Which freebl code do we actually want to use on x64  CPUs ? The code that currently is ifdef'ed for _X86_ or not ?
I am afraid this will require some performance tests.
Upping priority due to FIPS. This needs to be resolved one way or the other by april 1.
Priority: P4 → P2
The purpose of these tests was strictly to compare whether defining _X86_ or not was beneficial on those 64-bit platforms.

Conclusions :
- on Solaris x64, it's a wash
- on Linux, there is a very slight bias towards _X86_ being helpful, but I'm not sure the difference is statistically signficant - _X86_ seems to give about 1% improvement on the sum of the bandwidth of all algorithms
- On Windows x64, defining _X86_ clearly helps, by about 3% overall. The single algorithm that benefits most is SHA-1, which goes from 385 MB/s to 427 MB/s

Note that I got past the _X86_ symbol conflicts with Windows headers easily since only win_rand.c includes those problematic headers, and we don't have any code in there that depends on the _X86_ macro.

I can attach a quick and dirty patch that turns on _X86_ for Win64. But ideally we should really use and define another symbol.
Note that the hardware is identical for the Windows and Solaris machines in the previous data sheet, Sun w2100z workstation with dual opterons 246 2.0 GHz CPUs.

The Linux machine is a v20z with dual Opteron 246 also. Overall fairly similar hardware. The individual algorithm performance differences are interesting - we can see that some compilers do better than others.

gcc fared best, next was MSVC 9, then studio 10.

The main reason Studio 12 lags behind the others is due to SHA-512. The SHA-512 optimization in this bug cannot be implemented with studio since it lacks inline assembly. The same is true for RC4 - there is an inline assembly optimization with gcc and MSVC, but not studio.

AES and RC2 are also notably slower with Studio 12, but it's not obvious why - it's not because of lack of assembly optimizations.

Interestingly, DES/3DES performance is best with studio 12, even though it doesn't have the benefit of the bswap inline assembler optimization in this bug that benefits gcc and MSVC.

This was meant to be a quick performance test to answer the question of defining _X86_ vs not defining _X86_, so I will stop looking at the data now.
Using a 128 KB input data file (created with mkfile 128kb test), this script executes in about 2 minutes on a dual Opteron and tests the performance of many common cipher and hash algorithms. It runs multi-threaded (2 threads). Modify as appropriate for Windows (output to \NUL instead of /dev/null).
This is what I used for my measurements.
This will allow _X86_ to also be defined on Solaris x64, which I have determined was not harmful to code that uses that macro.
Attachment #369231 - Flags: review?(nelson)
This defines _X86_ on Windows x64, and in the process improves the overall crypto/hash performance, especially SHA-1. I didn't do it through the Makefile since it would cause win_rand.c to break.
We probably want to have a subsequent cleanup patch that renames all of our use of _X86_ in freebl to some other new and more meaningful symbol.
But until we have agreement on what to call that symbol, at least this will bring the expected code path and best level of performance on Windows x64.
Attachment #369233 - Flags: review?(nelson)
Re: comment 30, I discovered that the Linux machine is running with Opteron 248, which are 2.2 GHz, rather than 2.0 . So, direct comparisons aren't possible between the Linux results and the others. The comparisons between Solaris and Windows remain valid since hardware is identical.
Regardless, this doesn't affect the results of whether we should set _X86_ or not.
Comment on attachment 369231 [details] [diff] [review]
Use DEFINES instead of OS_REL_CFLAGS (checked in)

Remember to update the following comment to reflect
the new meaning of _X86_ and how it's defined:

80 # some codes want _X86_ defined for intel CPUs.
81 # coreconf does this for windows, but not for Linux, FreeBSD, etc.

For example, _X86_ is now defined jointly by
coreconf/WIN32.mk (for x86) and lib/freebl/blapi.h
(for x64; see attachment 369233 [details] [diff] [review]).
Comment on attachment 369231 [details] [diff] [review]
Use DEFINES instead of OS_REL_CFLAGS (checked in)

r=nelson for this patch
Attachment #369231 - Flags: review?(nelson) → review+
Comment on attachment 369233 [details] [diff] [review]
Quick patch to define _X86_ on Windows x64

r-
blapi.h is included by files outside of freebl, but we only want
this #define to affect builds of files inside freebl.

There is another header file, blapii.h, for use by files within
freebl only, so you could do this there, but I have a counter
proposal that I think is even simpler.  It consists of two parts:

1) define _X86_ for Win64 in the freebl Makefile, which ensures 
that it will only affect freebl files, and
2) #undef _X86_ in win_rand.c, since (you wrote)

> since only win_rand.c includes those problematic headers, and 
> we don't have any code in there that depends on the _X86_ macro.
Attachment #369233 - Flags: review?(nelson) → review-
Comment on attachment 369231 [details] [diff] [review]
Use DEFINES instead of OS_REL_CFLAGS (checked in)

Checking in Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.100; previous revision: 1.99
done
Attachment #369231 - Attachment description: Use DEFINES instead of OS_REL_CFLAGS → Use DEFINES instead of OS_REL_CFLAGS (checked in)
Wan-Teh,

re: comment 34, I think that comment applies only if both patches are checked in, but Nelson gave r- to attachment 369233 [details] [diff] [review], so I checked in attachment 369231 [details] [diff] [review] as-is.

re: comment 36,
I like the new proposal, and will come up with a new patch.
I also updated the comments in the Makefile.
I needed a second ifdef since the first filter didn't match anything on Windows.
Attachment #369233 - Attachment is obsolete: true
Attachment #369408 - Flags: review?(nelson)
Attachment #369408 - Attachment is patch: true
Attachment #369408 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 369408 [details] [diff] [review]
Define _X86_ on Win64. Nelson's proposal

Julien, thanks a lot for adding a comment.

Perhaps you can just add x386 to the first ifeq?

  ifeq (,$(filter-out x86 x86_64 x386,$(CPU_ARCH)))
      DEFINES += -D_X86_
  endif

This simplifies the makefile code, at the cost of
having two -D_X86_ for 32-bit Windows.

Perhaps we should change coreconf to also CPU_ARCH=x86
or x86_64 for Windows.  The "x386" is purely historical;
I don't think it's required by anything.

+ifdef USE_64
+ifeq (,$(filter-out WINNT WIN95,$(OS_TARGET)))
+	DEFINES += -D_X86_
+endif
+endif

Strictly speaking, this would be incorrect for IA64
(Itanium).
Comment on attachment 369408 [details] [diff] [review]
Define _X86_ on Win64. Nelson's proposal

This seems like it will work.  Wan-Teh's alternative also seems potentially more elegant, but I don't know enough about "x386" to evaluate it thoroughly.  If you do, Feel free to use his alternative.
Attachment #369408 - Flags: review?(nelson) → review+
It looks like x386 comes from WIN32.mk . It works. 

Re: comment 40,

I don't think it matters that _X86_ is defined twice.

IA64 is already broken, if you look at other tests in the freebl Makefile. The combination of Windows and NSS_USE_64 is assumed to be x64. I think if somebody wants to fix that, they should open an IA64 bug. I suspect it won't be only place that's broken in NSS or Mozilla.
Attachment #369408 - Attachment is obsolete: true
Attachment #369423 - Flags: review?(nelson)
Attachment #369423 - Flags: review?(nelson) → review+
Comment on attachment 369423 [details] [diff] [review]
Shorter patch (checked in)

r+
Comment on attachment 369423 [details] [diff] [review]
Shorter patch (checked in)

Checking in Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.101; previous revision: 1.100
done
Checking in win_rand.c;
/cvsroot/mozilla/security/nss/lib/freebl/win_rand.c,v  <--  win_rand.c
new revision: 1.22; previous revision: 1.21
done
Attachment #369423 - Attachment description: Shorter patch → Shorter patch (checked in)
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
In bug 485527, Julien wrote that the checkin for this bug broke NSPR
for Win64.  That problem is now a blocker, P1 for Monday's release.
If we cannot do anything better, we may have to back out this change.
Assignee: m_kato → julien.pierre.boogz
Severity: enhancement → blocker
Status: RESOLVED → REOPENED
Priority: P2 → P1
Resolution: FIXED → ---
Nelson,

This change didn't "break NSPR for Win64", it accidentally broke freebl in Win64 due to a conflict of this patch with the NSPR header files which also depend on the _X86_ symbol . That symbol was not set on Win64 prior to attachment 369423 [details] [diff] [review] being checked in.

We can either back out attachment 369423 [details] [diff] [review], or check in the change proposed by Wan-Teh in bug 485527, which will fix this regression in freebl.

I would much prefer the later.
Should I check in my _X86_ => NSS_X86 renaming patch now?
Wan-Teh,

You have my approval, but I don't know what Nelson thinks.
FYI, I went back to my office Opteron machine and saw that RC4 was broken on it too. This doesn't make much sense since I tested the performance on 3/24. I know I was doing clobber builds of freebl every time, and it was clearly working then. I reran the perf script once with the broken tip, and I got very close performance numbers to what I reported on Win64 for all algorithms, except for RC4, which crashed now, but didn't then. I don't have any explanation for it. Perhaps some other change in freebl subsequently broke it, but I can't imagine what. I am going to pull a tree as of right after my checkin to see if I can reproduce it.

In any case, the patch was supposed to be performance neutral on Solaris x64, no-op on Linux x64, and improving the performance on Win64 by 3%. The performance improvements are there - except for RC4 which now crashes. Backing  out attachment 369423 [details] [diff] [review] would be an option, but I would prefer if attachment 369676 [details] [diff] [review] got a second review.
I have confirmed that it was definitely my checkin and not the combination of my checkin + something else that caused the breakage.
Whiteboard: FIPS → FIPS _X86_
Julien, we had a meeting today by conference call.
The consensus of all parties who participated was that we will back out
the change that broke Win64, spin 3.12.3, and then next week, when 
3.12.3 is RTMed, we will reconsider this for 3.12.4 along with your other
patch for bug 485527.  

So, please back out the changes for this bug that broke Win64, with all
due haste.
I backed out the checkin for this bug.
Checking in freebl/Makefile;    new revision: 1.103; previous revision: 1.102
Checking in freebl/win_rand.c;  new revision: 1.24;  previous revision: 1.23
Target Milestone: 3.12.3 → 3.12.4
The relevant changes are included in attachment 371948 [details] [diff] [review] which was checked in as part of bug 485527 . Marking FIXED again.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
This change breaks building firefox on FreeBSD/ppc.  With this change, _X86_ is defined when building firefox 3.0.13 on FreeBSD/ppc, causing bad asm code to be generated in sha_fast.c, from sha_fast.h.  The code included is (from sha_fast.h):

#if defined(_X86_) || defined(__x86_64__) || defined(__x86_64) 
static __inline__ PRUint32 swap4b(PRUint32 value)
{
    __asm__("bswap %0" : "+r" (value));
    return (value);
}
#define SHA_HTONL(x) swap4b(x)
#endif /* x86 family */

The resulting error is "Error: Unrecognized opcode: `bswap'"
Thanks for the bug report.  I believe the problem is here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/FreeBSD.mk&rev=1.11&mark=45,48#45

We should be able to fix it by using code similar to NetBSD.mk
and OpenBSD.mk:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/NetBSD.mk&rev=1.6&mark=45-46,48-49#45

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/OpenBSD.mk&rev=1.6&mark=46-47,49-50#46

I'm not sure whether "arch -s" or "uname -p" is more appropriate
for FreeBSD.  The requirement is to map
- i386, i686, etc. to x86
- x86_64, amd64 to x86_64

Could you submit a patch when you get it working?  Thanks.
Please file a new bug for this FreeBSD issue
New bug including patch resolving this for powerpc at #511227.
You need to log in before you can comment on or make changes to this bug.