-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[ADT] Simplify hash_combine (NFC) #159938
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.
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
[ADT] Simplify hash_combine (NFC) #159938
Conversation
We just started using a C++17 fold expression inside combine (llvm#159901). We still carry the style that was suitable for recursive calls of combine_data. Specifically, we keep passing several state variables as parameters of combine_data. Now that we no longer use recursion, this patch simplifies hash_combine_recursive_helper by making buffer_ptr, buffer_end, and length member variables while dropping the return value from hash_combine. This patch also names hash_combine_recursive_helper to hash_combine_helper. I will follow up to update comments that still mention "recursion".
|
@llvm/pr-subscribers-llvm-adt Author: Kazu Hirata (kazutakahirata) ChangesWe just started using a C++17 fold expression inside combine Now that we no longer use recursion, this patch simplifies I will follow up to update comments that still mention "recursion". Full diff: https://github.com/llvm/llvm-project/pull/159938.diff 1 Files Affected:
diff --git a/llvm/include/llvm/ADT/Hashing.h b/llvm/include/llvm/ADT/Hashing.h
index dffe3791e51c7..31d7be2714a20 100644
--- a/llvm/include/llvm/ADT/Hashing.h
+++ b/llvm/include/llvm/ADT/Hashing.h
@@ -483,8 +483,11 @@ namespace detail {
/// recursive combining of arguments used in hash_combine. It is particularly
/// useful at minimizing the code in the recursive calls to ease the pain
/// caused by a lack of variadic functions.
-struct hash_combine_recursive_helper {
+struct hash_combine_helper {
char buffer[64] = {};
+ char *buffer_ptr;
+ char *const buffer_end;
+ size_t length = 0;
hash_state state;
const uint64_t seed;
@@ -493,8 +496,9 @@ struct hash_combine_recursive_helper {
///
/// This sets up the state for a recursive hash combine, including getting
/// the seed and buffer setup.
- hash_combine_recursive_helper()
- : seed(get_execution_seed()) {}
+ hash_combine_helper()
+ : buffer_ptr(buffer), buffer_end(buffer + 64),
+ seed(get_execution_seed()) {}
/// Combine one chunk of data into the current in-flight hash.
///
@@ -502,9 +506,7 @@ struct hash_combine_recursive_helper {
/// the data. If the buffer is full, it hashes the buffer into its
/// hash_state, empties it, and then merges the new chunk in. This also
/// handles cases where the data straddles the end of the buffer.
- template <typename T>
- char *combine_data(size_t &length, char *&buffer_ptr, char *buffer_end,
- T data) {
+ template <typename T> void combine_data(T data) {
if (!store_and_advance(buffer_ptr, buffer_end, data)) {
// Check for skew which prevents the buffer from being packed, and do
// a partial store into the buffer to fill it. This is only a concern
@@ -535,19 +537,14 @@ struct hash_combine_recursive_helper {
partial_store_size))
llvm_unreachable("buffer smaller than stored type");
}
- return buffer_ptr;
}
/// Recursive, variadic combining method.
///
/// This function recurses through each argument, combining that argument
/// into a single hash.
- template <typename... Ts>
- hash_code combine(size_t length, char *buffer_ptr, char *buffer_end,
- const Ts &...args) {
- ((void)combine_data(length, buffer_ptr, buffer_end,
- get_hashable_data(args)),
- ...);
+ template <typename... Ts> hash_code combine(const Ts &...args) {
+ (combine_data(get_hashable_data(args)), ...);
// Finalize the hash by flushing any remaining data in the buffer.
//
@@ -584,10 +581,10 @@ struct hash_combine_recursive_helper {
/// The result is suitable for returning from a user's hash_value
/// *implementation* for their user-defined type. Consumers of a type should
/// *not* call this routine, they should instead call 'hash_value'.
-template <typename ...Ts> hash_code hash_combine(const Ts &...args) {
+template <typename... Ts> hash_code hash_combine(const Ts &...args) {
// Recursively hash each argument using a helper class.
- ::llvm::hashing::detail::hash_combine_recursive_helper helper;
- return helper.combine(0, helper.buffer, helper.buffer + 64, args...);
+ ::llvm::hashing::detail::hash_combine_helper helper;
+ return helper.combine(args...);
}
// Implementation details for implementations of hash_value overloads provided
|
Now that we have removed recursion in llvm#159901 and llvm#159938, this patch updates those comments that still mention recursion.
We just started using a C++17 fold expression inside combine
(#159901). We still carry the style that was suitable for recursive
calls of combine_data. Specifically, we keep passing several state
variables as parameters of combine_data.
Now that we no longer use recursion, this patch simplifies
hash_combine_recursive_helper by making buffer_ptr, buffer_end, and
length member variables while dropping the return value from
hash_combine. This patch also names hash_combine_recursive_helper to
hash_combine_helper.
I will follow up to update comments that still mention "recursion".