Commit 8da62a1
committed
Merge bitcoin/bitcoin#29263: serialization: c++20 endian/byteswap/clz modernization
86b7f28 serialization: use internal endian conversion functions (Cory Fields)
432b18c serialization: detect byteswap builtins without autoconf tests (Cory Fields)
297367b crypto: replace CountBits with std::bit_width (Cory Fields)
52f9bba crypto: replace non-standard CLZ builtins with c++20's bit_width (Cory Fields)
Pull request description:
This replaces #28674, #29036, and #29057. Now ready for testing and review.
Replaces platform-specific endian and byteswap functions. This is especially useful for kernel, as it means that our deep serialization code no longer requires bitcoin-config.h.
I apologize for the size of the last commit, but it's hard to avoid making those changes at once.
All platforms now use our internal functions rather than libc or platform-specific ones, with the exception of MSVC.
Sadly, benchmarking showed that not all compilers are capable of detecting and optimizing byteswap functions, so compiler builtins are instead used where possible. However, they're now detected via macros rather than autoconf checks.
This[ matches how libc++ implements std::byteswap for c++23](https://github.com/llvm/llvm-project/blob/main/libcxx/include/__bit/byteswap.h#L26).
I suggest we move/rename `compat/endian.h`, but I left that out of this PR to avoid bikeshedding.
#29057 pointed out some irregularities in benchmarks. After messing with various compilers and configs for a few weeks with these changes, I'm of the opinion that we can't win on every platform every time, so we should take the code that makes sense going forward. That said, if any real-world slowdowns are caused here, we should obviously investigate.
ACKs for top commit:
maflcko:
ACK 86b7f28 📘
fanquake:
ACK 86b7f28 - we can finish pruning out the __builtin_clz* checks/usage once the minisketch code has been updated. This is more good cleanup pre-CMake & for the kernal.
Tree-SHA512: 715a32ec190c70505ffbce70bfe81fc7b6aa33e376b60292e801f60cf17025aabfcab4e8c53ebb2e28ffc5cf4c20b74fe3dd8548371ad772085c13aec8b7970eFile tree
11 files changed
+118
-324
lines changed- src
- compat
- crypto
- test
- fuzz
- util
11 files changed
+118
-324
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
958 | 958 | | |
959 | 959 | | |
960 | 960 | | |
961 | | - | |
| 961 | + | |
962 | 962 | | |
963 | 963 | | |
964 | 964 | | |
| |||
973 | 973 | | |
974 | 974 | | |
975 | 975 | | |
976 | | - | |
977 | | - | |
978 | | - | |
979 | | - | |
980 | | - | |
981 | | - | |
982 | | - | |
983 | | - | |
984 | | - | |
985 | | - | |
986 | | - | |
987 | | - | |
988 | 976 | | |
989 | 977 | | |
990 | 978 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
9 | | - | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | | - | |
13 | 13 | | |
14 | | - | |
15 | | - | |
16 | | - | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
19 | 35 | | |
20 | | - | |
21 | | - | |
22 | | - | |
23 | | - | |
| 36 | + | |
24 | 37 | | |
| 38 | + | |
| 39 | + | |
25 | 40 | | |
26 | | - | |
| 41 | + | |
| 42 | + | |
27 | 43 | | |
28 | | - | |
29 | | - | |
| 44 | + | |
30 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
31 | 49 | | |
| 50 | + | |
32 | 51 | | |
33 | | - | |
34 | 52 | | |
35 | | - | |
36 | | - | |
| 53 | + | |
37 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
38 | 58 | | |
39 | 59 | | |
| 60 | + | |
40 | 61 | | |
41 | | - | |
42 | 62 | | |
43 | | - | |
44 | | - | |
| 63 | + | |
45 | 64 | | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
46 | 68 | | |
47 | 69 | | |
48 | 70 | | |
| |||
51 | 73 | | |
52 | 74 | | |
53 | 75 | | |
| 76 | + | |
54 | 77 | | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | 78 | | |
59 | 79 | | |
0 commit comments