Skip to content

Commit 03c5b00

Browse files
fanquakefanquake
committed
Merge bitcoin#29085: refactor: C++20: Use std::rotl
6044628 crypto, hash: replace custom rotl32 with std::rotl (Fabian Jahr) Pull request description: While exploring some C++20 changes and checking against our code I found this potential improvement: 1. We can replace our custom implementation of `rotl32` in crypto/chacha20 with `std::rotl` from the [new `bit` header](https://en.cppreference.com/w/cpp/header/bit). ACKs for top commit: fanquake: ACK 6044628 Tree-SHA512: db55b366f20fca2ef62e5f10a838f8a709d531678c35c1dba20898754029c788a2fd47995208ed6d187cf814109a7ca397bc2c301504500aee79da04c95d6895
2 parents 3d52ced + 6044628 commit 03c5b00

File tree

5 files changed

+46
-57
lines changed

5 files changed

+46
-57
lines changed

Diff for: ‎src/crypto/chacha20.cpp

+5-6
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,14 @@
1111
#include <span.h>
1212

1313
#include <algorithm>
14+
#include <bit>
1415
#include <string.h>
1516

16-
constexpr static inline uint32_t rotl32(uint32_t v, int c) { return (v << c) | (v >> (32 - c)); }
17-
1817
#define QUARTERROUND(a,b,c,d) \
19-
a += b; d = rotl32(d ^ a, 16); \
20-
c += d; b = rotl32(b ^ c, 12); \
21-
a += b; d = rotl32(d ^ a, 8); \
22-
c += d; b = rotl32(b ^ c, 7);
18+
a += b; d = std::rotl(d ^ a, 16); \
19+
c += d; b = std::rotl(b ^ c, 12); \
20+
a += b; d = std::rotl(d ^ a, 8); \
21+
c += d; b = std::rotl(b ^ c, 7);
2322

2423
#define REPEAT10(a) do { {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; } while(0)
2524

Diff for: ‎src/crypto/sha3.cpp

+30-35
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,10 @@
1111

1212
#include <algorithm>
1313
#include <array> // For std::begin and std::end.
14+
#include <bit>
1415

1516
#include <stdint.h>
1617

17-
// Internal implementation code.
18-
namespace
19-
{
20-
uint64_t Rotl(uint64_t x, int n) { return (x << n) | (x >> (64 - n)); }
21-
} // namespace
22-
2318
void KeccakF(uint64_t (&st)[25])
2419
{
2520
static constexpr uint64_t RNDC[24] = {
@@ -41,38 +36,38 @@ void KeccakF(uint64_t (&st)[25])
4136
bc2 = st[2] ^ st[7] ^ st[12] ^ st[17] ^ st[22];
4237
bc3 = st[3] ^ st[8] ^ st[13] ^ st[18] ^ st[23];
4338
bc4 = st[4] ^ st[9] ^ st[14] ^ st[19] ^ st[24];
44-
t = bc4 ^ Rotl(bc1, 1); st[0] ^= t; st[5] ^= t; st[10] ^= t; st[15] ^= t; st[20] ^= t;
45-
t = bc0 ^ Rotl(bc2, 1); st[1] ^= t; st[6] ^= t; st[11] ^= t; st[16] ^= t; st[21] ^= t;
46-
t = bc1 ^ Rotl(bc3, 1); st[2] ^= t; st[7] ^= t; st[12] ^= t; st[17] ^= t; st[22] ^= t;
47-
t = bc2 ^ Rotl(bc4, 1); st[3] ^= t; st[8] ^= t; st[13] ^= t; st[18] ^= t; st[23] ^= t;
48-
t = bc3 ^ Rotl(bc0, 1); st[4] ^= t; st[9] ^= t; st[14] ^= t; st[19] ^= t; st[24] ^= t;
39+
t = bc4 ^ std::rotl(bc1, 1); st[0] ^= t; st[5] ^= t; st[10] ^= t; st[15] ^= t; st[20] ^= t;
40+
t = bc0 ^ std::rotl(bc2, 1); st[1] ^= t; st[6] ^= t; st[11] ^= t; st[16] ^= t; st[21] ^= t;
41+
t = bc1 ^ std::rotl(bc3, 1); st[2] ^= t; st[7] ^= t; st[12] ^= t; st[17] ^= t; st[22] ^= t;
42+
t = bc2 ^ std::rotl(bc4, 1); st[3] ^= t; st[8] ^= t; st[13] ^= t; st[18] ^= t; st[23] ^= t;
43+
t = bc3 ^ std::rotl(bc0, 1); st[4] ^= t; st[9] ^= t; st[14] ^= t; st[19] ^= t; st[24] ^= t;
4944

5045
// Rho Pi
5146
t = st[1];
52-
bc0 = st[10]; st[10] = Rotl(t, 1); t = bc0;
53-
bc0 = st[7]; st[7] = Rotl(t, 3); t = bc0;
54-
bc0 = st[11]; st[11] = Rotl(t, 6); t = bc0;
55-
bc0 = st[17]; st[17] = Rotl(t, 10); t = bc0;
56-
bc0 = st[18]; st[18] = Rotl(t, 15); t = bc0;
57-
bc0 = st[3]; st[3] = Rotl(t, 21); t = bc0;
58-
bc0 = st[5]; st[5] = Rotl(t, 28); t = bc0;
59-
bc0 = st[16]; st[16] = Rotl(t, 36); t = bc0;
60-
bc0 = st[8]; st[8] = Rotl(t, 45); t = bc0;
61-
bc0 = st[21]; st[21] = Rotl(t, 55); t = bc0;
62-
bc0 = st[24]; st[24] = Rotl(t, 2); t = bc0;
63-
bc0 = st[4]; st[4] = Rotl(t, 14); t = bc0;
64-
bc0 = st[15]; st[15] = Rotl(t, 27); t = bc0;
65-
bc0 = st[23]; st[23] = Rotl(t, 41); t = bc0;
66-
bc0 = st[19]; st[19] = Rotl(t, 56); t = bc0;
67-
bc0 = st[13]; st[13] = Rotl(t, 8); t = bc0;
68-
bc0 = st[12]; st[12] = Rotl(t, 25); t = bc0;
69-
bc0 = st[2]; st[2] = Rotl(t, 43); t = bc0;
70-
bc0 = st[20]; st[20] = Rotl(t, 62); t = bc0;
71-
bc0 = st[14]; st[14] = Rotl(t, 18); t = bc0;
72-
bc0 = st[22]; st[22] = Rotl(t, 39); t = bc0;
73-
bc0 = st[9]; st[9] = Rotl(t, 61); t = bc0;
74-
bc0 = st[6]; st[6] = Rotl(t, 20); t = bc0;
75-
st[1] = Rotl(t, 44);
47+
bc0 = st[10]; st[10] = std::rotl(t, 1); t = bc0;
48+
bc0 = st[7]; st[7] = std::rotl(t, 3); t = bc0;
49+
bc0 = st[11]; st[11] = std::rotl(t, 6); t = bc0;
50+
bc0 = st[17]; st[17] = std::rotl(t, 10); t = bc0;
51+
bc0 = st[18]; st[18] = std::rotl(t, 15); t = bc0;
52+
bc0 = st[3]; st[3] = std::rotl(t, 21); t = bc0;
53+
bc0 = st[5]; st[5] = std::rotl(t, 28); t = bc0;
54+
bc0 = st[16]; st[16] = std::rotl(t, 36); t = bc0;
55+
bc0 = st[8]; st[8] = std::rotl(t, 45); t = bc0;
56+
bc0 = st[21]; st[21] = std::rotl(t, 55); t = bc0;
57+
bc0 = st[24]; st[24] = std::rotl(t, 2); t = bc0;
58+
bc0 = st[4]; st[4] = std::rotl(t, 14); t = bc0;
59+
bc0 = st[15]; st[15] = std::rotl(t, 27); t = bc0;
60+
bc0 = st[23]; st[23] = std::rotl(t, 41); t = bc0;
61+
bc0 = st[19]; st[19] = std::rotl(t, 56); t = bc0;
62+
bc0 = st[13]; st[13] = std::rotl(t, 8); t = bc0;
63+
bc0 = st[12]; st[12] = std::rotl(t, 25); t = bc0;
64+
bc0 = st[2]; st[2] = std::rotl(t, 43); t = bc0;
65+
bc0 = st[20]; st[20] = std::rotl(t, 62); t = bc0;
66+
bc0 = st[14]; st[14] = std::rotl(t, 18); t = bc0;
67+
bc0 = st[22]; st[22] = std::rotl(t, 39); t = bc0;
68+
bc0 = st[9]; st[9] = std::rotl(t, 61); t = bc0;
69+
bc0 = st[6]; st[6] = std::rotl(t, 20); t = bc0;
70+
st[1] = std::rotl(t, 44);
7671

7772
// Chi Iota
7873
bc0 = st[0]; bc1 = st[1]; bc2 = st[2]; bc3 = st[3]; bc4 = st[4];

Diff for: ‎src/crypto/siphash.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@
44

55
#include <crypto/siphash.h>
66

7-
#define ROTL(x, b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b))))
7+
#include <bit>
88

99
#define SIPROUND do { \
10-
v0 += v1; v1 = ROTL(v1, 13); v1 ^= v0; \
11-
v0 = ROTL(v0, 32); \
12-
v2 += v3; v3 = ROTL(v3, 16); v3 ^= v2; \
13-
v0 += v3; v3 = ROTL(v3, 21); v3 ^= v0; \
14-
v2 += v1; v1 = ROTL(v1, 17); v1 ^= v2; \
15-
v2 = ROTL(v2, 32); \
10+
v0 += v1; v1 = std::rotl(v1, 13); v1 ^= v0; \
11+
v0 = std::rotl(v0, 32); \
12+
v2 += v3; v3 = std::rotl(v3, 16); v3 ^= v2; \
13+
v0 += v3; v3 = std::rotl(v3, 21); v3 ^= v0; \
14+
v2 += v1; v1 = std::rotl(v1, 17); v1 ^= v2; \
15+
v2 = std::rotl(v2, 32); \
1616
} while (0)
1717

1818
CSipHasher::CSipHasher(uint64_t k0, uint64_t k1)

Diff for: ‎src/hash.cpp

+4-8
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,9 @@
77
#include <crypto/common.h>
88
#include <crypto/hmac_sha512.h>
99

10+
#include <bit>
1011
#include <string>
1112

12-
inline uint32_t ROTL32(uint32_t x, int8_t r)
13-
{
14-
return (x << r) | (x >> (32 - r));
15-
}
16-
1713
unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vDataToHash)
1814
{
1915
// The following is MurmurHash3 (x86_32), see https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp
@@ -31,11 +27,11 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vData
3127
uint32_t k1 = ReadLE32(blocks + i*4);
3228

3329
k1 *= c1;
34-
k1 = ROTL32(k1, 15);
30+
k1 = std::rotl(k1, 15);
3531
k1 *= c2;
3632

3733
h1 ^= k1;
38-
h1 = ROTL32(h1, 13);
34+
h1 = std::rotl(h1, 13);
3935
h1 = h1 * 5 + 0xe6546b64;
4036
}
4137

@@ -55,7 +51,7 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vData
5551
case 1:
5652
k1 ^= tail[0];
5753
k1 *= c1;
58-
k1 = ROTL32(k1, 15);
54+
k1 = std::rotl(k1, 15);
5955
k1 *= c2;
6056
h1 ^= k1;
6157
}

Diff for: ‎test/sanitizer_suppressions/ubsan

-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ implicit-signed-integer-truncation:crypto/
6969
implicit-unsigned-integer-truncation:crypto/
7070
shift-base:arith_uint256.cpp
7171
shift-base:crypto/
72-
shift-base:ROTL32
7372
shift-base:streams.h
7473
shift-base:FormatHDKeypath
7574
shift-base:xoroshiro128plusplus.h

0 commit comments

Comments
 (0)