Skip to content

Commit

Permalink
[FIX] More elegant solution, probably
Browse files Browse the repository at this point in the history
  • Loading branch information
eseiler committed Oct 10, 2023
1 parent 819b6a4 commit 6055d7d
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
6 changes: 5 additions & 1 deletion include/hibf/sketch/hyperloglog.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ class hyperloglog
*/
void add(std::string_view const sv);

// Note: Sometimes, LTO optimizes the function body away. In particular, when value is in a register.
// Using const & is more or less the same as using `asm inline("" : : "m"(value));`, but it can still optimize
// for memory addresses and registers when inlining, whereas the asm will always create a memory address.

/*!\brief Adds an unsigned 64-bit integer to the estimator.
* \param[in] value unsigned integer to add
*/
void add(uint64_t const value);
void add(uint64_t const & value);

/*!\brief Estimates cardinality value.
* \returns Estimated cardinality value.
Expand Down
4 changes: 1 addition & 3 deletions src/sketch/hyperloglog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,8 @@ void hyperloglog::add(std::string_view const sv)
M_[index] = std::max(rank, M_[index]);
}

void hyperloglog::add(uint64_t const value)
void hyperloglog::add(uint64_t const & value)
{
// chopper hibf_statistics, gcc13 + lto: overzealous optimization results in wrong results
asm volatile("" : : "r,m"(value) : "memory");
uint64_t const hash = XXH3_64bits(&value, sizeof(uint64_t));
// the first b_ bits are used to distribute the leading zero counts along M_
uint64_t const index = hash >> (64 - b_);
Expand Down

0 comments on commit 6055d7d

Please sign in to comment.