Avoid overly-wide multiplications in 5x52 field mul/sqr

[peterdettman]

Speeds up bench_ecdh, bench_sign, bench_verify relative to master by 5+% at -O3, haswell.

[sipa]

As I understand it, this does two things:

• Adds some casts to inform the compiler that a 64x64->128 multiplication rather than a 128x64->128 multiplication is being done.
• Changes how some 128-bit values are split across variables, so that there are more shifts of exactly 64 bits (which translate to no-ops).

Do you ever run out of ideas for optimization? ;)

[sipa]

I measure a 5.9% speedup for ecdsa_verify on AMD Ryzen 2950X with clock fixed at 2.2 GHz (Linux x86_64, GCC 9.3, -O2, gmp enabled, endomorphism enabled, asm disabled). Before this PR the asm code was tied with the no-asm code; after, the no-asm code clearly wins (this isn't surprising, as the asm code implements the old algorithm, but still).

[sipa]

ACK b53e0cd

Verified the new equation and bounds by hand, ran 10000 iterations of the unit tests, and 1000 iterations of the exhaustive tests (with randomization from #808) on it.

[sipa]

I made the same changes to asm code in https://github.com/sipa/secp256k1/commits/202009_cleanup_5x52_mul, but it's only a ~2% speedup for the with-asm code (so the no-asm code is still faster) indicating that this code change enables more shuffling around than I could directly apply.

We could just take the GCC 9.3 generated code, and turn it into asm source code, or we could just drop the asm code. @gmaxwell also suggests to me that we could replace the asm code with a faster BMI2-enabled version, which wouldn't work on every x86_64 CPU. EDIT: compiling with -mbmi2 slows it down, though perhaps a well-written handrolled BMI2 version can be faster.

[elichai]

Cool trick!

On my machine (i9-9980HK @ 2.4GHz) with: ./configure --enable-experimental --enable-module-ecdh --enable-module-recovery --disable-openssl-tests --with-asm=no
Before:

$ ./bench_verify 
ecdsa_verify: min 95.1us / avg 95.2us / max 95.4us
$ ./bench_verify 
ecdsa_verify: min 95.2us / avg 96.3us / max 97.8us
$ ./bench_sign 
ecdsa_sign: min 68.7us / avg 69.1us / max 71.1us
$ ./bench_sign 
ecdsa_sign: min 68.7us / avg 68.8us / max 68.9us
$ ./bench_ecdh 
ecdh: min 108us / avg 109us / max 109us
$ ./bench_ecdh 
ecdh: min 108us / avg 109us / max 110us

After:

$ ./bench_verify 
ecdsa_verify: min 93.1us / avg 93.4us / max 94.4us
$ ./bench_verify 
ecdsa_verify: min 93.1us / avg 93.2us / max 93.3us
$ ./bench_sign 
ecdsa_sign: min 67.7us / avg 68.1us / max 69.8us
$ ./bench_sign 
ecdsa_sign: min 68.1us / avg 69.0us / max 72.2us
$ ./bench_ecdh 
ecdh: min 106us / avg 107us / max 109us
$ ./bench_ecdh 
ecdh: min 106us / avg 106us / max 108us

Seems to be ~2% improvement here.

But with CFLAGS=-O3 ./configure --enable-experimental --enable-module-ecdh --enable-module-recovery --disable-openssl-tests --with-asm=no
Before:

$ ./bench_verify 
ecdsa_verify: min 90.2us / avg 90.6us / max 92.0us
$ ./bench_verify 
ecdsa_verify: min 90.5us / avg 90.7us / max 91.0us
$ ./bench_sign 
ecdsa_sign: min 67.5us / avg 68.0us / max 69.3us
$ ./bench_sign 
ecdsa_sign: min 67.4us / avg 67.6us / max 67.7us
$ ./bench_ecdh 
ecdh: min 99.7us / avg 102us / max 108us
$ ./bench_ecdh 
ecdh: min 99.5us / avg 99.9us / max 101us

After:

$ ./bench_verify 
ecdsa_verify: min 88.7us / avg 89.9us / max 91.8us
$ ./bench_verify 
ecdsa_verify: min 88.9us / avg 89.6us / max 90.8us
$ ./bench_sign 
ecdsa_sign: min 66.8us / avg 66.9us / max 67.1us
$ ./bench_sign 
ecdsa_sign: min 66.7us / avg 67.0us / max 68.4us
$ ./bench_ecdh 
ecdh: min 96.1us / avg 97.1us / max 98.9us
$ ./bench_ecdh 
ecdh: min 95.9us / avg 96.2us / max 97.7us

Seems to be ~2-4% improvement here on -O3.

[sipa]

@elichai Feel like benchmarking the asm code too (with my branch linked above)?

[elichai]

@elichai Feel like benchmarking the asm code too (with my branch linked above)?

Here:
I verified that I still reproduce my past result(which I do), and then tested current master with ./configure --enable-experimental --enable-module-ecdh --enable-module-recovery --disable-openssl-tests --with-asm=yes

$ ./bench_verify
ecdsa_verify: min 92.0us / avg 92.3us / max 93.6us
$ ./bench_verify
ecdsa_verify: min 92.1us / avg 92.2us / max 92.5us
$ ./bench_sign
ecdsa_sign: min 66.0us / avg 66.8us / max 67.9us
$ ./bench_sign
ecdsa_sign: min 66.0us / avg 66.4us / max 67.3us
$ ./bench_ecdh
ecdh: min 104us / avg 105us / max 107us
$ ./bench_ecdh
ecdh: min 104us / avg 105us / max 106us

on @sipa's branch:

$ ./bench_verify
ecdsa_verify: min 90.8us / avg 91.1us / max 91.4us
$ ./bench_verify
ecdsa_verify: min 90.8us / avg 91.3us / max 92.9us
$ ./bench_sign
ecdsa_sign: min 65.4us / avg 65.8us / max 66.9us
$ ./bench_sign
ecdsa_sign: min 65.5us / avg 65.9us / max 66.6us
$ ./bench_ecdh
ecdh: min 103us / avg 103us / max 105us
$ ./bench_ecdh
ecdh: min 103us / avg 103us / max 104us

master with -O3:

$ ./bench_verify
ecdsa_verify: min 93.1us / avg 93.2us / max 93.2us
$ ./bench_verify
ecdsa_verify: min 93.0us / avg 93.5us / max 94.9us
$ ./bench_sign
ecdsa_sign: min 65.3us / avg 65.8us / max 67.2us
$ ./bench_sign
ecdsa_sign: min 65.3us / avg 65.8us / max 67.0us
$ ./bench_ecdh
ecdh: min 104us / avg 104us / max 105us
$ ./bench_ecdh
ecdh: min 104us / avg 105us / max 106us

@sipa's branch with -O3:

$ ./bench_verify
ecdsa_verify: min 92.2us / avg 92.4us / max 94.2us
$ ./bench_verify
ecdsa_verify: min 92.1us / avg 92.2us / max 92.2us
$ ./bench_sign
ecdsa_sign: min 65.0us / avg 65.3us / max 65.9us
$ ./bench_sign
ecdsa_sign: min 65.1us / avg 65.6us / max 66.5us
$ ./bench_ecdh
ecdh: min 102us / avg 102us / max 103us
$ ./bench_ecdh
ecdh: min 102us / avg 103us / max 104us

I don't get how regular -O3 gives the best result in everything except signing

[elichai]

Using this: #726 (comment) it seems like -O3 and -O2 still generates the exact same asm routine for field_5x52_int128_impl.

(this PR and master do generate different asm's)

[elichai]

I went over all the asm differences between -O2 and -O3 when compiling filed_5x52_impl.h with int128 in gcc and they differ only in these functions:

secp256k1_fe_normalize_var
secp256k1_fe_clear
secp256k1_fe_cmp_var
secp256k1_fe_add
secp256k1_fe_cmov
secp256k1_fe_storage_cmov

the main interesting thing is secp256k1_fe_add which -O3 seems to be optimizing with SSE instructions: https://godbolt.org/z/f7xa35

sadly trying to compile and test with -mno-sse breaks gcc

[sipa]

FWIW, those are SSE/SSE2 instructions, which are guaranteed to be available on x86_64.

[peterdettman]

@elichai FYI my results were with the endomorphism enabled, which may be skewing the relative improvement percentage compared to having it off. From my testing it appears this tweak is improving fe_mul more than fe_sqr, which is somewhat counter-intuitive since the same operations were saved for each and fe_sqr has less overall.

[sipa]

FWIW, my benchmarks were also with endomorphism enabled.

[gmaxwell]

Pretty much all benchmarking now should be endomorphism enabled unless there is a specific reason not to test with it. Endomorphism should become default on (or even on and not configurable) within a month.

[real-or-random]

ACK b53e0cd I've inspected the diff and run the tests without asm for a CPU day

[sipa]

FWIW, on a 64-bit ARM system (developerbox, Cortex-A53):

• before this PR: ecdsa_verify: min 481us / avg 481us / max 481us
• after this PR: ecdsa_verify: min 470us / avg 470us / max 470us