Closed
Bug 272327
Opened 20 years ago
Closed 19 years ago
AMD64 assembly multiply routine for freebl / mpi
Categories
(NSS :: Libraries, enhancement, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
3.10
People
(Reporter: julien.pierre, Assigned: julien.pierre)
References
Details
Attachments
(1 file, 4 obsolete files)
29.50 KB,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
The Solaris crypto group at Sun has decided to donate optimizations assembly optimizations for the bignum library for the AMD64 instruction set. This code roughly doubles the RSA performance . On a v20z with opteron model 248 CPU at 2.2 GHz, using gcc and -O3 optimizations on Solaris 10, rsaperf goes from 585 ops/s for the pure C implementation, to 1186 ops/s with this code . This is for 1024 bit RSA keys, with rsaperf running single-threaded, ie. it is a per-CPU ops number ... Patch forthcoming later today.
Assignee | ||
Comment 1•20 years ago
|
||
Assignee | ||
Comment 2•20 years ago
|
||
Comment on attachment 167713 [details] [diff] [review] Use 64x64 multiply instruction I realize we may want to do additional cleanup, but I'd like to get this checked in to the tip so we can do performance runs from nightly tip builds .
Attachment #167713 -
Flags: review?(nelson)
Assignee | ||
Comment 3•20 years ago
|
||
Attachment #167713 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #167713 -
Flags: review?(nelson)
Assignee | ||
Comment 4•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #167718 -
Flags: superreview?(rrelyea0264)
Attachment #167718 -
Flags: review?(nelson)
Assignee | ||
Updated•20 years ago
|
Attachment #167719 -
Flags: superreview?
Attachment #167719 -
Flags: review?(nelson)
Assignee | ||
Updated•20 years ago
|
Severity: normal → enhancement
Priority: -- → P1
Target Milestone: --- → 3.9.5
Assignee | ||
Updated•20 years ago
|
Attachment #167719 -
Flags: superreview?(nelson)
Attachment #167719 -
Flags: superreview?
Attachment #167719 -
Flags: review?(saul.edwards.bugs)
Attachment #167719 -
Flags: review?(nelson)
Assignee | ||
Updated•20 years ago
|
Attachment #167718 -
Flags: superreview?(rrelyea)
Attachment #167718 -
Flags: superreview?(nelson)
Attachment #167718 -
Flags: review?(saul.edwards.bugs)
Attachment #167718 -
Flags: review?(nelson)
Assignee | ||
Comment 5•20 years ago
|
||
Guys, it's been 2 weeks since I asked for review on this. I'm sure you'll have some review comments as the code isn't the cleanest, especially the C part. I would like to see this checked in before the holiday break. Thanks !
Comment 6•20 years ago
|
||
Comment on attachment 167718 [details] [diff] [review] forgot the main files ... 3 issues with this patch: a) please globally rename the new functions from big_mul_* to s_mpv_mul_* or mpi_mul_* or some other prefix that is already used within MPI. b) All of NSS has been converted to use the new "tri-license", which allows use under MPL, GPL, or LGPL. But the new files added here seem to be using the older dual license, which permits use only under MPL and GPL, but not LGPL. I believe all of NSS needs to be liccensed under the same license. c) Looks like you did a global search and replace in one file, replacing all slashes (/) with hashes (#). So now there are lines like the following, that need to be fixed. >+# the License at http:##www.mozilla.org#MPL#
Attachment #167718 -
Flags: superreview?(nelson) → superreview-
Comment 7•20 years ago
|
||
Comment on attachment 167719 [details] and another one ... >/* > * Copyright 2004 Sun Microsystems, Inc. All rights reserved. > * Use is subject to license terms. > */ mozilla cannot accept the file with this license. This file needs the MPL tri-license. > d = big_mul_add_vec64(c, a, a_len, b); Please change this name to use on MPI's existing prefixes. I'd suggest s_mpv_ for the following reasons: a) s_ implies a private mpi-internal function, not publicly callable b) mpv_ prefix is used for functions tht operate directly on vectors (arrays) of mp_digit. mpi_ is used for functions that operate on mp_int's.
Attachment #167719 -
Flags: superreview?(nelson) → superreview-
Updated•20 years ago
|
Attachment #167718 -
Flags: review?(saul.edwards.bugs)
Updated•20 years ago
|
Attachment #167719 -
Flags: review?(saul.edwards.bugs)
Assignee | ||
Comment 8•20 years ago
|
||
*** Bug 256664 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 9•19 years ago
|
||
- update the license - fix the types to use mpi types - rename big_mul functions to s_mpv_ - fix comments in mpi_amd64_gas.s
Attachment #167718 -
Attachment is obsolete: true
Attachment #167719 -
Attachment is obsolete: true
Attachment #175487 -
Flags: review?(nelson)
Assignee | ||
Comment 10•19 years ago
|
||
- add -fPIC to ASFLAGS in Makefile for the gcc builds on Solaris and Linux - add -DMPI_64 to the AMD64SOLARIS target in target.mk for the standalone mpi build - remove erroneous comments in assembly source
Assignee | ||
Updated•19 years ago
|
Attachment #175509 -
Flags: review?(nelson)
Assignee | ||
Updated•19 years ago
|
Attachment #175487 -
Attachment is obsolete: true
Attachment #175487 -
Flags: review?(nelson)
Comment 11•19 years ago
|
||
Comment on attachment 175509 [details] [diff] [review] improved patch r=nelson. Looks ready.
Attachment #175509 -
Flags: review?(nelson) → review+
Assignee | ||
Comment 12•19 years ago
|
||
Thanks, Nelson. I checked in this patch to the tip. Checking in Makefile; /cvsroot/mozilla/security/nss/lib/freebl/Makefile,v <-- Makefile new revision: 1.60; previous revision: 1.59 done Checking in mpi/mpi-priv.h; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi-priv.h,v <-- mpi-priv.h new revision: 1.18; previous revision: 1.17 done Checking in mpi/mpi-test.c; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi-test.c,v <-- mpi-test.c new revision: 1.13; previous revision: 1.12 done RCS file: /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi_amd64.c,v done Checking in mpi/mpi_amd64.c; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi_amd64.c,v <-- mpi_amd64.c initial revision: 1.1 done RCS file: /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi_amd64_gas.s,v done Checking in mpi/mpi_amd64_gas.s; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi_amd64_gas.s,v <-- mpi_amd64_gas.s initial revision: 1.1 done RCS file: /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi_amd64_sun.s,v done Checking in mpi/mpi_amd64_sun.s; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpi_amd64_sun.s,v <-- mpi_amd64_sun.s initial revision: 1.1 done Checking in mpi/target.mk; /cvsroot/mozilla/security/nss/lib/freebl/mpi/target.mk,v <-- target.mk new revision: 1.3; previous revision: 1.2 done
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 13•19 years ago
|
||
The target milestone 3.9.5 is wrong because NSS 3.9.5 has been released.
Assignee | ||
Comment 14•19 years ago
|
||
Thanks for pointing that out, Wan-Teh. Changed to 3.10 .
Target Milestone: 3.9.5 → 3.10
You need to log in
before you can comment on or make changes to this bug.
Description
•