Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions include/cuco/detail/probing_scheme_base.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

#pragma once

#include <cuco/detail/__config>

Comment thread
srinivasyadav18 marked this conversation as resolved.
Outdated
#include <cstdint>

namespace cuco {
Expand Down
4 changes: 2 additions & 2 deletions include/cuco/detail/probing_scheme_impl.inl
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ __host__ __device__ constexpr auto linear_probing<CGSize, Hash>::operator()(
{
using size_type = typename Extent::value_type;
return detail::probing_iterator<Extent>{
cuco::detail::sanitize_hash<size_type>(hash_(probe_key) + g.thread_rank()) % upper_bound,
cuco::detail::sanitize_hash<size_type>(hash_(probe_key), g.thread_rank()) % upper_bound,
cg_size,
upper_bound};
}
Expand Down Expand Up @@ -164,7 +164,7 @@ __host__ __device__ constexpr auto double_hashing<CGSize, Hash1, Hash2>::operato
{
using size_type = typename Extent::value_type;
return detail::probing_iterator<Extent>{
cuco::detail::sanitize_hash<size_type>(hash1_(probe_key) + g.thread_rank()) % upper_bound,
cuco::detail::sanitize_hash<size_type>(hash1_(probe_key), g.thread_rank()) % upper_bound,
static_cast<size_type>(
(cuco::detail::sanitize_hash<size_type>(hash2_(probe_key)) % (upper_bound / cg_size - 1) +
1) *
Expand Down
38 changes: 35 additions & 3 deletions include/cuco/detail/utils.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <cuco/detail/bitwise_compare.cuh>

#include <cuda/std/array>
Comment thread
srinivasyadav18 marked this conversation as resolved.
#include <cuda/std/bit>
#include <cuda/std/cmath>
#include <cuda/std/type_traits>
Expand Down Expand Up @@ -81,6 +82,16 @@ struct slot_is_filled {
}
};

template <typename SizeType, typename HashType>
__host__ __device__ constexpr SizeType to_positive(HashType hash)
{
if constexpr (cuda::std::is_signed_v<SizeType>) {
return cuda::std::abs(static_cast<SizeType>(hash));
} else {
return static_cast<SizeType>(hash);
}
}

/**
* @brief Converts a given hash value into a valid (positive) size type.
*
Expand All @@ -92,12 +103,33 @@ struct slot_is_filled {
template <typename SizeType, typename HashType>
__host__ __device__ constexpr SizeType sanitize_hash(HashType hash) noexcept
{
if constexpr (cuda::std::is_signed_v<SizeType>) {
return cuda::std::abs(static_cast<SizeType>(hash));
if constexpr (cuda::std::is_same_v<HashType, cuda::std::array<std::uint64_t, 2>>) {
#if !defined(CUCO_HAS_INT128)
static_assert(false,
"CUCO_HAS_INT128 undefined. Need unsigned __int128 type when sanitizing "
"cuda::std::array<std::uint64_t, 2>");
#endif
unsigned __int128 ret{};
memcpy(&ret, &hash, sizeof(unsigned __int128));
return to_positive<SizeType>(static_cast<SizeType>(ret));
} else {
return static_cast<SizeType>(hash);
return to_positive<SizeType>(hash);
}
}

/**
* @brief Converts a given hash value and cg_rank, into a valid (positive) size type.
*
* @tparam SizeType The target type
* @tparam HashType The input type
*
* @return Converted hash value
*/
template <typename SizeType, typename HashType>
__host__ __device__ constexpr SizeType sanitize_hash(HashType hash, std::uint32_t cg_rank) noexcept
Comment thread
srinivasyadav18 marked this conversation as resolved.
Outdated
{
return sanitize_hash<SizeType>(sanitize_hash<SizeType>(hash) + cg_rank);
Copy link
Copy Markdown
Collaborator

@sleeepyjack sleeepyjack Jun 13, 2024

Choose a reason for hiding this comment

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

There exists a scenario where this approach fails and it's the reason CI in my initial draft PR failed.
Consider the following example:

  • SizeType is int32_t aka a signed type
  • The value of sanitize_hash<SizeType>(hash) is very close to numeric_limits<SizeType>::max()

In this scenario, if we compute sanitize_hash<SizeType>(hash) + cg_rank there's chance the result oxceeds numeric_limits<SizeType>::max() which would result in a signed integer overflow which is undefined behavior under the C++ abstract machine. Thus the compiler is free to produce any garbage code around this call.

To solve this we need check if sanitize_hash<SizeType>(hash) > (numeric_limits<SizeType>::max() - group.size()) (be careful with > and >=, I'm infamous for my off-by-one errors) and then compute the wrapped-around value manually in case this expression evaluates to true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the detailed explanation!
I think 3ae1afe covers this case now.

}

} // namespace detail
} // namespace cuco
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ ConfigureTest(STATIC_MAP_TEST
static_map/custom_type_test.cu
static_map/duplicate_keys_test.cu
static_map/erase_test.cu
static_map/hash_test.cu
static_map/heterogeneous_lookup_test.cu
static_map/insert_and_find_test.cu
static_map/insert_or_assign_test.cu
Expand Down
72 changes: 72 additions & 0 deletions tests/static_map/hash_test.cu
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright (c) 2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <test_utils.hpp>

#include <cuco/hash_functions.cuh>
#include <cuco/static_map.cuh>

#include <thrust/device_vector.h>
#include <thrust/functional.h>
#include <thrust/iterator/counting_iterator.h>
#include <thrust/iterator/transform_iterator.h>

#include <catch2/catch_template_test_macros.hpp>

using size_type = std::size_t;

template <typename Key, typename Hash>
void test_hash_function()
{
using Value = int64_t;

constexpr size_type num_keys{400};

auto map = cuco::static_map<Key,
Value,
cuco::extent<size_type>,
cuda::thread_scope_device,
thrust::equal_to<Key>,
cuco::linear_probing<1, Hash>,
cuco::cuda_allocator<std::byte>,
cuco::storage<2>>{
num_keys, cuco::empty_key<Key>{-1}, cuco::empty_value<Value>{-1}};

auto keys_begin = thrust::counting_iterator<Key>(1);

auto pairs_begin = thrust::make_transform_iterator(
keys_begin, cuda::proclaim_return_type<cuco::pair<Key, Value>>([] __device__(auto i) {
return cuco::pair<Key, Value>(i, i);
}));

thrust::device_vector<bool> d_keys_exist(num_keys);

map.insert(pairs_begin, pairs_begin + num_keys);

REQUIRE(map.size() == num_keys);

map.contains(keys_begin, keys_begin + num_keys, d_keys_exist.begin());

REQUIRE(cuco::test::all_of(d_keys_exist.begin(), d_keys_exist.end(), thrust::identity{}));
}

TEMPLATE_TEST_CASE_SIG("static_map hash tests", "", ((typename Key)), (int32_t), (int64_t))
{
test_hash_function<Key, cuco::murmurhash3_32<Key>>();
test_hash_function<Key, cuco::murmurhash3_x64_128<Key>>();
test_hash_function<Key, cuco::xxhash_32<Key>>();
test_hash_function<Key, cuco::xxhash_64<Key>>();
}