This PR adds support for ARMv8 SHA2 Intrinsics.
Concept ACK!
detection when the feature can be used
On Linux (the only system we care about for ARM, i guess), the following would be the way to do detection:
#include <sys/auxv.h>
#include <asm/hwcap.h>
…
#ifdef __arm__
/* ARM 32 bit */
if (getauxval(AT_HWCAP2) & HWCAP2_SHA2) {
have_arm_shani = true;
}
#endif
#ifdef __aarch64__
/* ARM 64 bit */
if (getauxval(AT_HWCAP) & HWCAP_SHA2) {
have_arm_shani = true;
}
#endif
Note that the capability bit is on a different HWCAP word on 32 and 64 bit (dunno if you even want to support 32 bit here).
the following would be the way to do detection:
Added in f7dd1ef
On commit f7dd1efae715593f5c9ff8186d518d25d1c9023c
On a Linux aarch64 Cortex-A53 system with:
$ cat /proc/cpuinfo
processor : 0
BogoMIPS : 200.00
Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant : 0x0
CPU part : 0xd03
CPU revision : 4
which I presume means it has the necessary SHA2 extensions.
The GCC 9.3.0 compiler used supports the extensions (crypto/libbitcoin_crypto_arm_shani.a is being built):
checking for x86 SHA-NI intrinsics... no
checking whether C++ compiler accepts -march=armv8-a+crc+crypto... yes
checking whether C++ compiler accepts -march=armv8-a+crc+crypto... (cached) yes
checking for AArch64 CRC32 intrinsics... yes
checking for AArch64 SHA-NI intrinsics... yes
Still, the extension doesn't seem to be detected. debug.log says:
2022-01-20T20:37:05Z Using the 'standard' SHA256 implementation
On c0849fc4fd9aa7e70973445a1b962a9270fd859c:
2022-01-20T22:15:44Z Using the 'arm_shani(1way)' SHA256 implementation
This PR (c0849fc4fd9aa7e70973445a1b962a9270fd859c):
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 2.56 | 390,886,099.70 | 0.9% | 0.03 | SHA256
| 10.27 | 97,329,584.86 | 0.0% | 0.01 | SHA256D64_1024
| 14.60 | 68,478,670.64 | 0.4% | 0.01 | SHA256_32b
On master (e3ce019667fba2ec50a59814a26566fb67fa9125):
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 15.69 | 63,715,155.54 | 0.0% | 0.17 | SHA256
| 43.42 | 23,029,615.98 | 0.5% | 0.03 | SHA256D64_1024
| 41.67 | 23,995,549.21 | 0.1% | 0.01 | SHA256_32b
On Linux (the only system we care about for ARM, i guess)
M1 macs would like a word with you.
Speaking of m1, I was able to compile this locally on my m1 pro 10 core, ./configure realized that SHA2 intrinsics could be used. See benchmarks below.
on https://github.com/bitcoin/bitcoin/commit/c0849fc4fd9aa7e70973445a1b962a9270fd859c
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.47 | 2,148,996,364.97 | 0.3% | 0.01 | SHA256
| 1.48 | 676,354,727.08 | 0.3% | 0.01 | SHA256D64_1024
| 1.15 | 873,060,380.08 | 0.1% | 0.01 | SHA256_32b
on master
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 3.10 | 322,550,263.01 | 1.3% | 0.03 | SHA256
| 9.26 | 107,941,088.30 | 0.7% | 0.01 | SHA256D64_1024
| 6.22 | 160,743,377.26 | 1.6% | 0.01 | SHA256_32b
Speaking of m1, I was able to compile this locally on my m1 pro 10 core, ./configure realized that SHA2 intrinsics could be used. See benchmarks below.
Yes, support for Apple Silicon is included in this PR.
45 | + 46 | + // Load state 47 | + STATE0 = vld1q_u32(&s[0]); 48 | + STATE1 = vld1q_u32(&s[4]); 49 | + 50 | + const uint8x16_t* input32 = reinterpret_cast<const uint8x16_t*>(chunk);
I'm a bit concerned this is not well-defined. We can't guarantee that chunk has an alignment that is compatible with the uint8x16_t type, and even if it is, whether such a reinterpretation is permitted would be highly architecture-specific (of course, this is already architecture-specific code). Do you know of documentation that specifically permits this?
If not, I'd use this patch:
diff --git a/src/crypto/sha256_arm_shani.cpp b/src/crypto/sha256_arm_shani.cpp
index c051d87042..a783be9068 100644
--- a/src/crypto/sha256_arm_shani.cpp
+++ b/src/crypto/sha256_arm_shani.cpp
@@ -47,8 +47,6 @@ void Transform(uint32_t* s, const unsigned char* chunk, size_t blocks)
STATE0 = vld1q_u32(&s[0]);
STATE1 = vld1q_u32(&s[4]);
- const uint8x16_t* input32 = reinterpret_cast<const uint8x16_t*>(chunk);
-
while (blocks--)
{
// Save state
@@ -56,10 +54,14 @@ void Transform(uint32_t* s, const unsigned char* chunk, size_t blocks)
CDGH_SAVE = STATE1;
// Load and convert input chunk to Big Endian
- MSG0 = vreinterpretq_u32_u8(vrev32q_u8(*input32++));
- MSG1 = vreinterpretq_u32_u8(vrev32q_u8(*input32++));
- MSG2 = vreinterpretq_u32_u8(vrev32q_u8(*input32++));
- MSG3 = vreinterpretq_u32_u8(vrev32q_u8(*input32++));
+ MSG0 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 0)));
+ MSG1 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 16)));
+ MSG2 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 32)));
+ MSG3 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 48)));
+ chunk += 64;
// Original implemenation preloaded message and constant addition which was 1-3% slower.
// Now included as first step in quad round code saving one Q Neon register
Better safe than sorry - applied in f06f46c
I do not see any performance hit with this change.
Tested c0849fc4fd9aa7e70973445a1b962a9270fd859c on Mac mini (M1, 2020):
% time ./src/bitcoind -datadir=/Users/hebasto/SHANI -assumevalid=0 -stopatheight=719700 -prune=550
2022-01-21T07:44:20Z Bitcoin Core version v22.99.0-c0849fc4fd9a (release build)
2022-01-21T07:44:20Z Validating signatures for all blocks.
2022-01-21T07:44:20Z Setting nMinimumChainWork=00000000000000000000000000000000000000001fa4663bbbe19f82de910280
2022-01-21T07:44:20Z Prune configured to target 550 MiB on disk for block and undo files.
2022-01-21T07:44:20Z Using the 'arm_shani(1way)' SHA256 implementation
...
2022-01-21T22:38:17Z Shutdown: done
./src/bitcoind -datadir=/Users/hebasto/SHANI -assumevalid=0 -prune=550 149587.28s user 11456.17s system 300% cpu 14:53:56.52 total
UPDATE. The same for the master branch (e3ce019667fba2ec50a59814a26566fb67fa9125):
% time ./src/bitcoind -datadir=/Users/hebasto/MASTER -assumevalid=0 -stopatheight=719700 -prune=550
2022-01-21T22:49:25Z Bitcoin Core version v22.99.0-e3ce019667fb (release build)
2022-01-21T22:49:25Z Validating signatures for all blocks.
2022-01-21T22:49:25Z Setting nMinimumChainWork=00000000000000000000000000000000000000001fa4663bbbe19f82de910280
2022-01-21T22:49:25Z Prune configured to target 550 MiB on disk for block and undo files.
2022-01-21T22:49:25Z Using the 'standard' SHA256 implementation
...
2022-01-22T14:37:08Z Shutdown: done
./src/bitcoind -datadir=/Users/hebasto/MASTER -assumevalid=0 -prune=550 174110.07s user 11526.30s system 326% cpu 15:47:43.83 total
51 min or 6% faster IBD.
See https://github.com/sipa/bitcoin/commits/pr24115, which adds a 2-way 64-byte optimized variant. On my Cortex-A53 It's roughly a 2x speedup for the SHA256D64_1024 benchmark (relevant for Merkle root computation) compared to this PR. For more modern architectures I could imagine it's more:
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 2.60 | 384,105,263.28 | 0.3% | 0.03 | SHA256
| 5.35 | 187,019,153.94 | 0.1% | 0.01 | SHA256D64_1024
| 14.61 | 68,437,280.69 | 0.0% | 0.01 | SHA256_32b
For reference, master again:
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 15.69 | 63,715,155.54 | 0.0% | 0.17 | SHA256
| 43.42 | 23,029,615.98 | 0.5% | 0.03 | SHA256D64_1024
| 41.67 | 23,995,549.21 | 0.1% | 0.01 | SHA256_32b
@sipa's branch on m1:
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.46 | 2,174,603,243.64 | 0.6% | 0.01 | SHA256
| 0.95 | 1,053,985,898.82 | 0.8% | 0.01 | SHA256D64_1024
| 1.15 | 871,857,965.44 | 0.4% | 0.01 | SHA256_32b
previous results on c0849
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.47 | 2,148,996,364.97 | 0.3% | 0.01 | SHA256
| 1.48 | 676,354,727.08 | 0.3% | 0.01 | SHA256D64_1024
| 1.15 | 873,060,380.08 | 0.1% | 0.01 | SHA256_32b
639 | +#ifdef __linux__ 640 | +#ifdef __arm__ // 32-bit 641 | + if (getauxval(AT_HWCAP2) & HWCAP2_SHA2) { 642 | + have_arm_shani = true; 643 | + } 644 | +#endif
I'm not sure if bitcoin core has a preference documented somewhere, but I prefer documenting what the endif is associated with such as
#endif // __arm__
I checked all the other files in src/crypto and we use the endif comments only for the include guards.
I confirm the numbers on M1:
before @sipa's improvements (f06f46cd5e2d732669e70fbfeacd7bf9b9ac8815):
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.45 | 2,198,307,083.71 | 0.3% | 0.01 | SHA256
| 1.49 | 671,303,457.11 | 0.3% | 0.01 | SHA256D64_1024
| 1.15 | 866,670,334.07 | 0.2% | 0.01 | SHA256_32b
after @sipa's improvements (0e7299508af17014a906a951ad9b4ecfcc891bb1):
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.46 | 2,197,198,571.82 | 0.3% | 0.01 | SHA256
| 0.94 | 1,059,405,097.65 | 0.3% | 0.01 | SHA256D64_1024
| 1.17 | 857,291,152.82 | 0.5% | 0.01 | SHA256_32b
I'm not able to build this branch on m1 at the moment
config.status: creating libbitcoinconsensus.pc
config.status: creating Makefile
config.status: creating src/Makefile
config.status: creating doc/man/Makefile
config.status: creating share/setup.nsi
config.status: creating share/qt/Info.plist
config.status: creating test/config.ini
config.status: creating contrib/devtools/split-debug.sh
config.status: creating src/config/bitcoin-config.h
config.status: src/config/bitcoin-config.h is unchanged
config.status: executing depfiles commands
config.status: executing libtool commands
cd . && /bin/sh /Users/pasta/workspace/bitcoin/build-aux/missing automake-1.16 --foreign
/bin/sh: /Users/pasta/workspace/bitcoin/build-aux/missing: No such file or directory
make: *** [Makefile.in] Error 1
I just checked out sipa's branch here: https://github.com/sipa/bitcoin/commit/0e7299508af17014a906a951ad9b4ecfcc891bb1 and compilation worked trivially
@prusnak @PastaPastaPasta Perhaps you want to also benchmark with the two last commits removed (so at "Optimization: precompute a few 3rd transform intermediaries"). Whether the last two help may be very architecture-dependent. For me they contribute a ~30% speedup, but maytbe on M1 that is not the case.
@sipa benchmark of 38ed75f8b9fdb60ab45d07f3d4069a9c9cb636d4 Optimization: precompute a few 3rd transform intermediaries on M1:
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 0.46 | 2,163,137,327.85 | 0.0% | 0.01 | SHA256
| 1.28 | 780,225,941.01 | 0.3% | 0.01 | SHA256D64_1024
| 1.18 | 850,467,547.93 | 0.2% | 0.01 | SHA256_32b
The improvement of using 0e72995 is there also for M1.
Looks like the 2-way version is a clear win on M1 as well, thanks!
Approach ACK 1cfacecf8203f775f5827fa225a6dccf7a28cdef.
It seems commits need some cleanup (reorder? squash?) for the nice commit history, no?
Feel free to squash/rebase wherever you feel it is useful.
Feel free to squash/rebase wherever you feel it is useful.
Thanks! Squashed and rebased => 1f140783e1fbf818d4a6045464db3903af426bd0
0 | @@ -0,0 +1,898 @@ 1 | +// Copyright (c) 2018-2020 The Bitcoin Core developers
was this moved from somewhere? Otherwise it should probably be updated
Addresssed in the latest rebase
14 | +#include <cstddef> 15 | +#include <arm_acle.h> 16 | +#include <arm_neon.h> 17 | + 18 | +namespace { 19 | +static const uint32_t K[64] =
nit: I'd prefer this being a constexpr std::array
Addresssed in the latest rebase
198 | + 199 | +namespace sha256d64_arm_shani { 200 | +void Transform_2way(unsigned char* output, const unsigned char* input) 201 | +{ 202 | + /* Initial state. */ 203 | + static const uint32_t INIT[8] = {
nit: same, constexpr std::array, and below
Addresssed in the latest rebase
652 | + have_arm_shani = true; 653 | + } 654 | +#endif 655 | +#endif 656 | + 657 | +#if defined(__APPLE__)
a6ae9ef1626c73253aa8e6c8e406f3ee18b8017f
style nit: #ifdef __linux__ vs #ifdef __aarch64__ vs #if defined(__APPLE__) looks inconsistent. Mind choosing one style for new code?
Addressed in 9560981b0dda342b79e25067132dcb37e8d9e27b
ACK a6ae9ef1626c73253aa8e6c8e406f3ee18b8017f (first two commits, won't ACK code change in /src/crypto/ because of lack of my expertise).
re-ACK 9560981b0dda342b79e25067132dcb37e8d9e27b (first two commits only).
15 | +#include <cstddef> 16 | +#include <arm_acle.h> 17 | +#include <arm_neon.h> 18 | + 19 | +namespace { 20 | +static constexpr std::array<uint32_t, 64> K =
May want to put an alignas(uint32x4_t) on this and other constants that are read using vld1q_u32 (better alignment may matter on some platforms).
Addressed in fe180d53c9035dfcec4211182ac954bd95ccaa60 + 195eb2fbe3754efe2ba31fbadb0fa77e801ce8bf
Tested 5202c6dd7695f66325638e15861468999ec834c1 on Mac mini (M1, 2020):
% time ./src/bitcoind -datadir=/Users/hebasto/SHANI-2 -assumevalid=0 -stopatheight=719700 -prune=550
2022-01-23T18:25:19Z Bitcoin Core version v22.99.0-5202c6dd7695 (release build)
2022-01-23T18:25:19Z Validating signatures for all blocks.
2022-01-23T18:25:19Z Setting nMinimumChainWork=00000000000000000000000000000000000000001fa4663bbbe19f82de910280
2022-01-23T18:25:19Z Prune configured to target 550 MiB on disk for block and undo files.
2022-01-23T18:25:19Z Using the 'arm_shani(1way,2way)' SHA256 implementation
...
2022-01-24T08:16:46Z Shutdown: done
./src/bitcoind -datadir=/Users/hebasto/SHANI-2 -assumevalid=0 -prune=550 147477.18s user 11980.05s system 319% cpu 13:51:28.21 total
117 min or 12% faster IBD than the master branch. Also see #24115 (comment).
652 | + have_arm_shani = true; 653 | + } 654 | +#endif 655 | +#endif 656 | + 657 | +#if defined(__APPLE__)
Can you use MAC_OSX here to match our other macOS defines.
Addressed in 3af2f3f5024a43bacceff4aabfd108a8cd971d1f
ACK 4abca94b1e12792e821c2169ff7bee05f35ca284
Compiled and benchmarked on two aarch64 SBC:
Rock Pi 4 (RK3399, Dual Cortex-A72 and Quad Cortex-A53):
master bd482b3
| ns/byte | byte/s | err% | ins/byte | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------:|:----------
| 6.64 | 150,596,444.75 | 0.0% | 30.02 | 0.07 | SHA256
| 18.89 | 52,942,682.86 | 0.0% | 82.25 | 0.01 | SHA256D64_1024
| 15.95 | 62,706,213.16 | 0.0% | 70.72 | 0.01 | SHA256_32b
| ns/byte | byte/s | err% | ins/byte | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------:|:----------
| 1.56 | 642,517,847.54 | 0.9% | 1.75 | 0.02 | SHA256
| 2.70 | 370,040,371.53 | 0.0% | 4.38 | 0.01 | SHA256D64_1024
| 5.13 | 194,811,160.90 | 1.2% | 13.59 | 0.01 | SHA256_32b
Firefly RK3566-ROC-PC (Quad-core Cortex-A55)
master bd482b3
| ns/byte | byte/s | err% | ins/byte | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|---------------:|--------:|----------:|:----------
| 9.06 | 110,429,040.01 | 0.1% | 30.02 | 0.02 | 0.1% | 0.10 | SHA256
| 24.94 | 40,094,485.36 | 0.9% | 82.25 | 0.05 | 0.3% | 0.02 | SHA256D64_1024
| 22.83 | 43,792,465.97 | 0.1% | 70.72 | 1.09 | 0.0% | 0.01 | SHA256_32b
| ns/byte | byte/s | err% | ins/byte | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|---------------:|--------:|----------:|:----------
| 1.49 | 672,648,756.27 | 0.5% | 1.75 | 0.02 | 0.1% | 0.02 | SHA256
| 3.10 | 322,551,749.73 | 1.1% | 4.38 | 0.02 | 0.2% | 0.01 | SHA256D64_1024
| 7.53 | 132,832,412.14 | 0.0% | 13.59 | 1.09 | 0.0% | 0.01 | SHA256_32b
ACK
May I propose this to be added to 23.0 Milestone?
Two questions: the machine that builds the aarch64 binaries, does that have these extensions? Or in other words, if this is merged, will the aarch64 binaries have this included?
And has someone tested the case of running this build on a device without the crypto extensions, f.e. a Raspberry Pi?
ACK 3abdb593618e62d2c0f0bcc29e19bbadbf31e84c. I have not reviewed whether the macOS feature detection works. Also, some of the code is my own.
I did verify that the x86 SHA-NI keeps working (benchmark + debug.log on my Ryzen 5950X system indicate the SHA instructions are being used). I've also verified the instruction sequence, and reviewed the constants used cursorily. I conjecture that it is impossible to create anything resembling the SHA-2 transformation's flow using these instructions that passes unit tests in a non-trivial test set, but is still wrong in others. Simply because these instructions are so high-level, any modification would greatly affect the outcome of nearly every input.
Also rename AArch64 intrinsics to ARMv8 intrinsics
as these are not necessarily limited to 64-bit
Co-Authored-By: Rauli Kumpulainen <rauliweb@gmail.com>
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
4a309ef27036065f787330a50659c85323f78f7b5d3c69a79e3eca232f4d3e55 guix-build-aaa1d03d3ace/output/aarch64-linux-gnu/SHA256SUMS.part
cfedbd51f5bf65d57fe9200e22e56f3a38f44308eea565b201483811996d957b guix-build-aaa1d03d3ace/output/aarch64-linux-gnu/bitcoin-aaa1d03d3ace-aarch64-linux-gnu-debug.tar.gz
85c9195cf594fbbf3ce6c6b76370f746dd4342c793b54b8b5bdb650a53761eaf guix-build-aaa1d03d3ace/output/aarch64-linux-gnu/bitcoin-aaa1d03d3ace-aarch64-linux-gnu.tar.gz
280234b6a20bbcddb847e0bbaf27f35ac30d292ea5dd096f8c8096220f8ef29a guix-build-aaa1d03d3ace/output/arm-linux-gnueabihf/SHA256SUMS.part
48c0fd0a6a4c1e86e942f6907804ed87527f329954e81e5bdc22beed83cbe6b3 guix-build-aaa1d03d3ace/output/arm-linux-gnueabihf/bitcoin-aaa1d03d3ace-arm-linux-gnueabihf-debug.tar.gz
c435b7a53606f33f9d23f28e6c98ce3d55f50f61fd67bf2b4ebe65261fe68aba guix-build-aaa1d03d3ace/output/arm-linux-gnueabihf/bitcoin-aaa1d03d3ace-arm-linux-gnueabihf.tar.gz
e0d61af7b471ba5135a848be6d4d6cf0caa8042dab9d929f6578de17e47e40c2 guix-build-aaa1d03d3ace/output/arm64-apple-darwin/SHA256SUMS.part
e9eda618bf90d5d1522005bd13bd296ae89aec9c3b0b2528b9c9f67e70178828 guix-build-aaa1d03d3ace/output/arm64-apple-darwin/bitcoin-aaa1d03d3ace-arm64-apple-darwin.tar.gz
a6540dcea7c2a2562edd0f2232f511da97339d388cdca686e084d03e381d6fa2 guix-build-aaa1d03d3ace/output/arm64-apple-darwin/bitcoin-aaa1d03d3ace-osx-unsigned.dmg
6d970ae7c94b78c5f97e978718862e85c3375c48d3b8ade6fafce39e7c8e0f0e guix-build-aaa1d03d3ace/output/arm64-apple-darwin/bitcoin-aaa1d03d3ace-osx-unsigned.tar.gz
cdbc9eb5281b14ecbecc2956a5a41709fda93762115ce3cee9516a68874676b8 guix-build-aaa1d03d3ace/output/dist-archive/bitcoin-aaa1d03d3ace.tar.gz
d063099449e40036d15ed5b023f602ba824edf303ecbe78bcd3f01feeabb535f guix-build-aaa1d03d3ace/output/powerpc64-linux-gnu/SHA256SUMS.part
3e6da4026d466039cd361e48a492ea780021bc0efb8e67e948097871ce4226cc guix-build-aaa1d03d3ace/output/powerpc64-linux-gnu/bitcoin-aaa1d03d3ace-powerpc64-linux-gnu-debug.tar.gz
669291a4767509f053bdd8789f4631ff6d28cd4ad62156105bc98cdb7d3a295a guix-build-aaa1d03d3ace/output/powerpc64-linux-gnu/bitcoin-aaa1d03d3ace-powerpc64-linux-gnu.tar.gz
868cbe0f73cd786d91dd6d1990550e6c9c673ef3d874c545150b6e030951a24d guix-build-aaa1d03d3ace/output/powerpc64le-linux-gnu/SHA256SUMS.part
90157bdc29262abd421535e4d6921e72aff7f3d43ca5634bc76598d8daf3a1ec guix-build-aaa1d03d3ace/output/powerpc64le-linux-gnu/bitcoin-aaa1d03d3ace-powerpc64le-linux-gnu-debug.tar.gz
8275665e19f85193de4e7e5ee7b451d6a4f8e414a33586e7066575812e878eda guix-build-aaa1d03d3ace/output/powerpc64le-linux-gnu/bitcoin-aaa1d03d3ace-powerpc64le-linux-gnu.tar.gz
2723b48d06e8217adb41bcc640417cba3470b234cce7815104e878866d775046 guix-build-aaa1d03d3ace/output/riscv64-linux-gnu/SHA256SUMS.part
f63822813587ec9e4bcc044c4b7918b1330d6b16be09f28bd95fedfd3dcdb147 guix-build-aaa1d03d3ace/output/riscv64-linux-gnu/bitcoin-aaa1d03d3ace-riscv64-linux-gnu-debug.tar.gz
3ec47d6968e2e430e3ff1629f07243f8589bd30406c0de916c2bbc6d5d88e0e8 guix-build-aaa1d03d3ace/output/riscv64-linux-gnu/bitcoin-aaa1d03d3ace-riscv64-linux-gnu.tar.gz
2120e8021edfe8170e318d82e44e405af68bb04c3fe3a3cd0407a17507cd74a0 guix-build-aaa1d03d3ace/output/x86_64-apple-darwin/SHA256SUMS.part
11b173cbeff7b20c717fd880446904eec2d33b30076348c0e9698e876f5be4a4 guix-build-aaa1d03d3ace/output/x86_64-apple-darwin/bitcoin-aaa1d03d3ace-osx-unsigned.dmg
af86643730d769ddf6635e9e240b50bd5b94aa576cf7450b4289d8ec1fc883ed guix-build-aaa1d03d3ace/output/x86_64-apple-darwin/bitcoin-aaa1d03d3ace-osx-unsigned.tar.gz
3c3591954aaf6b0557b74baf156f892b9b90ec63c35f3495889431afe0a17f93 guix-build-aaa1d03d3ace/output/x86_64-apple-darwin/bitcoin-aaa1d03d3ace-osx64.tar.gz
553d227d7e32c72390e259a46f37e4fd98f5781bd4b6d7ed8d83c5a9ba04f65b guix-build-aaa1d03d3ace/output/x86_64-linux-gnu/SHA256SUMS.part
479f3597f1577a33dad6634b97515f4ceac0ff163d64e7fa00b8685793d36231 guix-build-aaa1d03d3ace/output/x86_64-linux-gnu/bitcoin-aaa1d03d3ace-x86_64-linux-gnu-debug.tar.gz
c273a93458f2afa04c66d7e23346835dc6201f8e7b878ffbf661ae104c8746e7 guix-build-aaa1d03d3ace/output/x86_64-linux-gnu/bitcoin-aaa1d03d3ace-x86_64-linux-gnu.tar.gz
UPDATE: build artifacts are available in https://github.com/hebasto/artefacts/tree/master/pr24115/guix-build-aaa1d03d3ace/output
Two questions:
Systems used:
I performed the following tests:
checking for ARMv8 SHA-NI intrinsics... yesUsing the 'standard' SHA256 implementationchecking for ARMv8 SHA-NI intrinsics... yesUsing the 'arm_shani(1way,2way)' SHA256 implementationUsing the 'standard' SHA256 implementationUsing the 'arm_shani(1way,2way)' SHA256 implementationUsing the 'arm_shani(1way,2way)' SHA256 implementationUsing the 'standard' SHA256 implementationI think this proves the build mechanism and the runtime detection works as intended.
Concept ACK. I find it near-impossible to follow what sha256d64_arm_shani is doing, but that's mainly because our c++ TransformD64 is undocumented (introduced in #13191). In particular I don't understand how the algorithm follows from a single sha256. I assume it's an optimization. Otherwise they seem similar enough, with sha256d64_arm_shani splitting the input to take advantage of the 2-way instructions. And the tests pass :-)
@Sjors That's quite possibly worth documenting in general (for all D64 code).
What these functions do:
N*64 bytes buffer, and an output N*32 bytes buffer (N=1 for 1-way ,N=2 for 2-way, etc).SHA256(SHA256(input)) for each, and concatenate those outputs in the output buffer.A bit about SHA256's structure. SHA256(bytes) is really the following algorithm:
state = Transform(state, block), where Transform is the SHA256 transformation function at a high level.In case of SHA256(SHA256(64 bytes)), there are 3 Transforms being invoked:
There are 3 types of optimizations we can do in this case:
The individual commits in https://github.com/sipa/bitcoin/commits/pr24115 show the process.
Note that I don't think it's really required for verifying correctness to see these steps (otherwise I'd have argued for including them in this PR), but it may help understand how it came to be.
I think this is the step that confuses me:
The second continues on the resulting state, processing 64 bytes of padding. That padding is a constant (it's just a function of the length of the input).
If the first transform is the equivalent of a single sha256(64 bytes) and the third is the equivalent of a second sha256() on the 32 byte result of the first, what is the second transform doing?
I did this by simply writing the code up to this point, adding printf statements on these intermediaries, then turning the printed values into constants in the code and skipping their computation.
This is definitely worth documenting (can be another PR). Even nicer if we can generate the values in a Python script (for manual comparison, not code generation).
There are two SHA256 invocations:
Input is 64 bytes, which means it gets 64 bytes of padding (because the padding is always between 9 and 72 bytes long, and the result is always a multiple of 64).
For H2, SHA256(H1) just gets a 32-byte input, so it also only gets a 32-byte padding, and the result just needs one transform.
So we can write it this way:
The first transform is the inner one for H1, the second the outer one for H1. The third transform is the H2 one.
Ah that makes sense.
Input is 64 bytes, which means it gets 64 bytes of padding
I naively assumed a 64 byte message wasn't padded, but it is: https://datatracker.ietf.org/doc/html/rfc6234#section-4.1
IBD up to block 700000 on a Rock Pi 4a w/ NVMe SSD, assumevalid=0, dbcache=2000:
master (bd482b3f): 68H52M shani (4abca94): 65H29M
Improvement ~5%
I naively assumed a 64 byte message wasn't padded, but it is: https://datatracker.ietf.org/doc/html/rfc6234#section-4.1
Yes, it has to be. Otherwise you'd have a trivial 2nd preimage attack between hash(X) and hash(X || padding(len(X))), for non-multiple-of-64-bytes X.
<!--9cd9c72976c961c55c7acef8f6ba82cd-->
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
libbitcoinkernel by dongcarl)If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Code review and lightly tested ACK aaa1d03d3acebeb44fdd40a302f086aad3d329ce I have checked
60 | + MSG1 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 16))); 61 | + MSG2 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 32))); 62 | + MSG3 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 48))); 63 | + chunk += 64; 64 | + 65 | + // Original implemenation preloaded message and constant addition which was 1-3% slower.
typo: implemenation ==> implementation