Skip to content

Commit 2f0762f

Browse files
field: Remove x86_64 asm
Widely available versions of GCC and Clang beat our field asm on -O2. In particular, GCC 10.5.0, which is Bitcoin Core's current compiler for official x86_64 builds, produces code that is > 20% faster for fe_mul and > 10% faster for signature verification (see #726). These are the alternatives to this PR: We could replace our current asm with the fastest compiler output that we can find. This is potentially faster, but it has multiple drawbacks: - It's more coding work because it needs detailed benchmarks (e.g., with many compiler/options). - It's more review work because we need to deal with inline asm (including clobbers etc.) and there's a lack of experts reviewers in this area. - It's not unlikely that we'll fall behind again in a few compiler versions, and then we have to deal with this again, i.e., redo the benchmarks. Given our history here, I doubt that we'll revolve this timely. We could change the default of the asm build option to off. But this will also disable the scalar asm, which is still faster. We could split the build option into two separate options for field and scalar asm and only disable the field asm by default. But this adds complexity to the build and to the test matrix. My conclusion is that this PR gets the low-hanging fruit in terms of performance. It simplifies our code significantly. It's clearly an improvement, and it's very easy to review. Whether re-introducing better asm (whether from a compiler or from CryptOpt) is worth the hassle can be evaluated separately, and should not hold up this improvement. Solves #726.
1 parent e721039 commit 2f0762f

File tree

4 files changed

+1
-510
lines changed

4 files changed

+1
-510
lines changed

Makefile.am

-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ noinst_HEADERS += src/field_10x26_impl.h
3737
noinst_HEADERS += src/field_5x52.h
3838
noinst_HEADERS += src/field_5x52_impl.h
3939
noinst_HEADERS += src/field_5x52_int128_impl.h
40-
noinst_HEADERS += src/field_5x52_asm_impl.h
4140
noinst_HEADERS += src/modinv32.h
4241
noinst_HEADERS += src/modinv32_impl.h
4342
noinst_HEADERS += src/modinv64.h

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Implementation details
3333
* Expose only higher level interfaces to minimize the API surface and improve application security. ("Be difficult to use insecurely.")
3434
* Field operations
3535
* Optimized implementation of arithmetic modulo the curve's field size (2^256 - 0x1000003D1).
36-
* Using 5 52-bit limbs (including hand-optimized assembly for x86_64, by Diederik Huys).
36+
* Using 5 52-bit limbs
3737
* Using 10 26-bit limbs (including hand-optimized assembly for 32-bit ARM, by Wladimir J. van der Laan).
3838
* This is an experimental feature that has not received enough scrutiny to satisfy the standard of quality of this library but is made available for testing and review by the community.
3939
* Scalar operations

0 commit comments

Comments
 (0)