Skip to content

refactor: remove all c style casts in minisketch subtree #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

refactor: remove all c style casts in minisketch subtree #57

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64367c8
replace static_cast with braced / paren init where possible
PastaPastaPasta Dec 18, 2021
faf2849
replace static_cast with braced init where possible
PastaPastaPasta Dec 21, 2021
c3ef328
replace static_cast
PastaPastaPasta Jan 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/fields/clmul_common_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ template<typename I, int BITS, I MOD> NO_SANITIZE_MEMORY I MulWithClMulReduce(I
static constexpr I MASK = Mask<BITS, I>();

const __m128i MOD128 = _mm_cvtsi64_si128(MOD);
__m128i product = _mm_clmulepi64_si128(_mm_cvtsi64_si128((uint64_t)a), _mm_cvtsi64_si128((uint64_t)b), 0x00);
__m128i product = _mm_clmulepi64_si128(_mm_cvtsi64_si128(uint64_t(a)), _mm_cvtsi64_si128(uint64_t(b)), 0x00);
Copy link
Collaborator

@sipa sipa Dec 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all of these can be uint64_t{a} etc.

if (BITS <= 32) {
__m128i high1 = _mm_srli_epi64(product, BITS);
__m128i red1 = _mm_clmulepi64_si128(high1, MOD128, 0x00);
Expand Down Expand Up @@ -69,7 +69,7 @@ template<typename I, int BITS, int POS> NO_SANITIZE_MEMORY I MulTrinomial(I a, I
{
static constexpr I MASK = Mask<BITS, I>();

__m128i product = _mm_clmulepi64_si128(_mm_cvtsi64_si128((uint64_t)a), _mm_cvtsi64_si128((uint64_t)b), 0x00);
__m128i product = _mm_clmulepi64_si128(_mm_cvtsi64_si128(uint64_t(a)), _mm_cvtsi64_si128(uint64_t(b)), 0x00);
if (BITS <= 32) {
__m128i high1 = _mm_srli_epi64(product, BITS);
__m128i red1 = _mm_xor_si128(high1, _mm_slli_epi64(high1, POS));
Expand All @@ -92,7 +92,7 @@ template<typename I, int BITS, int POS> NO_SANITIZE_MEMORY I MulTrinomial(I a, I
return _mm_cvtsi128_si64(_mm_xor_si128(_mm_xor_si128(product, red1), red2)) & MASK;
}
} else {
const __m128i MOD128 = _mm_cvtsi64_si128(1 + (((uint64_t)1) << POS));
const __m128i MOD128 = _mm_cvtsi64_si128(1 + ((uint64_t{1}) << POS));
__m128i red1 = _mm_clmulepi64_si128(high1, MOD128, 0x00);
__m128i high2 = _mm_or_si128(_mm_srli_epi64(red1, BITS), _mm_srli_si128(_mm_slli_epi64(red1, 64 - BITS), 8));
__m128i red2 = _mm_xor_si128(high2, _mm_slli_epi64(high2, POS));
Expand Down Expand Up @@ -143,7 +143,7 @@ template<typename I, int B, I MOD, I (*MUL)(I, I), typename F, const F* SQR, con
Elem FromSeed(uint64_t seed) const {
uint64_t k0 = 0x434c4d554c466c64ull; // "CLMULFld"
uint64_t k1 = seed;
uint64_t count = ((uint64_t)B) << 32;
uint64_t count = (uint64_t(B)) << 32;
Copy link
Collaborator

@sipa sipa Dec 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be uint64_t{B}, and then doesn't need parenthesis around it anymore.

I ret;
do {
ret = O::Mask(I(SipHash(k0, k1, count++)));
Expand Down
2 changes: 1 addition & 1 deletion src/fields/generic_common_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ template<typename I, int B, uint32_t MOD, typename F, typename T, const F* SQR,
Elem FromSeed(uint64_t seed) const {
uint64_t k0 = 0x496e744669656c64ull; // "IntField"
uint64_t k1 = seed;
uint64_t count = ((uint64_t)B) << 32;
uint64_t count = (uint64_t(B)) << 32;
Elem ret;
do {
ret = O::Mask(I(SipHash(k0, k1, count++)));
Expand Down
4 changes: 2 additions & 2 deletions src/sketch_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ class SketchImpl final : public Sketch
auto poly = BerlekampMassey(all_syndromes, max_count, m_field);
if (poly.size() == 0) return -1;
if (poly.size() == 1) return 0;
if ((int)poly.size() > 1 + max_count) return -1;
if (int(poly.size()) > 1 + max_count) return -1;
std::reverse(poly.begin(), poly.end());
auto roots = FindRoots(poly, m_basis, m_field);
if (roots.size() == 0) return -1;
Expand All @@ -422,7 +422,7 @@ class SketchImpl final : public Sketch

void SetSeed(uint64_t seed) override
{
if (seed == (uint64_t)-1) {
if (seed == std::numeric_limits<uint64_t>::max()) {
m_basis = 1;
} else {
m_basis = m_field.FromSeed(seed);
Expand Down
6 changes: 3 additions & 3 deletions src/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ std::vector<Minisketch> CreateSketches(uint32_t bits, size_t capacity) {
if (Minisketch::ImplementationSupported(bits, impl)) {
CHECK(Minisketch::BitsSupported(bits));
ret.push_back(Minisketch(bits, impl, capacity));
CHECK((bool)ret.back());
CHECK(bool{ret.back()});
} else {
// implementation 0 must always work unless field size is disabled
CHECK(impl != 0 || !Minisketch::BitsSupported(bits));
Expand Down Expand Up @@ -274,7 +274,7 @@ int main(int argc, char** argv) {
try {
test_complexity = 0;
long long complexity = std::stoll(arg, &len);
if (complexity >= 1 && len == arg.size() && ((uint64_t)complexity <= std::numeric_limits<uint64_t>::max() >> 10)) {
if (complexity >= 1 && len == arg.size() && (static_cast<uint64_t>(complexity) <= std::numeric_limits<uint64_t>::max() >> 10)) {
test_complexity = complexity;
}
} catch (const std::logic_error&) {}
Expand All @@ -289,7 +289,7 @@ int main(int argc, char** argv) {
#else
const char* mode = "";
#endif
printf("Running libminisketch tests%s with complexity=%llu\n", mode, (unsigned long long)test_complexity);
printf("Running libminisketch tests%s with complexity=%llu\n", mode, static_cast<unsigned long long>(test_complexity));
Copy link
Collaborator

@sipa sipa Dec 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use (unsigned long long){test_complexity} here to avoid a C-style case (and prevent narrowing at the same time!).


TestComputeFunctions();

Expand Down