Skip to content

Commit

Permalink
Fix build failures with GCC 13 (#16488)
Browse files Browse the repository at this point in the history
Closes #16395

This PR resolves two types of compilation errors, allowing for successful builds with GCC 13:

- replacing the `cuco_allocator` strong type with an alias to fix a new build time check with GCC 13
- removing `std::move` when returning a temporary

Authors:
  - Yunsong Wang (https://github.com/PointKernel)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Mark Harris (https://github.com/harrism)

URL: #16488
  • Loading branch information
PointKernel authored Aug 5, 2024
1 parent 837dfe5 commit 8068a2d
Show file tree
Hide file tree
Showing 25 changed files with 195 additions and 193 deletions.
17 changes: 4 additions & 13 deletions cpp/include/cudf/detail/cuco_helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,10 @@ static double constexpr CUCO_DESIRED_LOAD_FACTOR = 0.5;
* later expects a standard C++ `Allocator` interface. This allocator helper provides a simple way
* to handle cuco memory allocation/deallocation with the given `stream` and the rmm default memory
* resource.
*
* @tparam T The allocator's value type.
*/
class cuco_allocator
: public rmm::mr::stream_allocator_adaptor<rmm::mr::polymorphic_allocator<char>> {
/// Default stream-ordered allocator type
using default_allocator = rmm::mr::polymorphic_allocator<char>;
/// The base allocator adaptor type
using base_type = rmm::mr::stream_allocator_adaptor<default_allocator>;

public:
/**
* @brief Constructs the allocator adaptor with the given `stream`
*/
cuco_allocator(rmm::cuda_stream_view stream) : base_type{default_allocator{}, stream} {}
};
template <typename T>
using cuco_allocator = rmm::mr::stream_allocator_adaptor<rmm::mr::polymorphic_allocator<T>>;

} // namespace cudf::detail
2 changes: 1 addition & 1 deletion cpp/include/cudf/detail/distinct_hash_join.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ struct distinct_hash_join {
cuda::thread_scope_device,
comparator_adapter<d_equal_type>,
probing_scheme_type,
cudf::detail::cuco_allocator,
cudf::detail::cuco_allocator<char>,
cuco_storage_type>;

bool _has_nulls; ///< true if nulls are present in either build table or probe table
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/cudf/detail/hash_reduce_by_row.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
namespace cudf::detail {

using hash_map_type = cuco::legacy::
static_map<size_type, size_type, cuda::thread_scope_device, cudf::detail::cuco_allocator>;
static_map<size_type, size_type, cuda::thread_scope_device, cudf::detail::cuco_allocator<char>>;

/**
* @brief The base struct for customized reduction functor to perform reduce-by-key with keys are
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/cudf/detail/join.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ struct hash_join {
cuco::static_multimap<hash_value_type,
cudf::size_type,
cuda::thread_scope_device,
cudf::detail::cuco_allocator,
cudf::detail::cuco_allocator<char>,
cuco::legacy::double_hashing<DEFAULT_JOIN_CG_SIZE, Hasher, Hasher>>;

hash_join() = delete;
Expand Down
14 changes: 7 additions & 7 deletions cpp/include/cudf_test/column_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1337,7 +1337,7 @@ class lists_column_wrapper : public detail::column_wrapper {
lists_column_wrapper(std::initializer_list<SourceElementT> elements) : column_wrapper{}
{
build_from_non_nested(
std::move(cudf::test::fixed_width_column_wrapper<T, SourceElementT>(elements).release()));
cudf::test::fixed_width_column_wrapper<T, SourceElementT>(elements).release());
}

/**
Expand All @@ -1361,7 +1361,7 @@ class lists_column_wrapper : public detail::column_wrapper {
lists_column_wrapper(InputIterator begin, InputIterator end) : column_wrapper{}
{
build_from_non_nested(
std::move(cudf::test::fixed_width_column_wrapper<T, SourceElementT>(begin, end).release()));
cudf::test::fixed_width_column_wrapper<T, SourceElementT>(begin, end).release());
}

/**
Expand All @@ -1386,7 +1386,7 @@ class lists_column_wrapper : public detail::column_wrapper {
: column_wrapper{}
{
build_from_non_nested(
std::move(cudf::test::fixed_width_column_wrapper<T, SourceElementT>(elements, v).release()));
cudf::test::fixed_width_column_wrapper<T, SourceElementT>(elements, v).release());
}

/**
Expand All @@ -1413,8 +1413,8 @@ class lists_column_wrapper : public detail::column_wrapper {
lists_column_wrapper(InputIterator begin, InputIterator end, ValidityIterator v)
: column_wrapper{}
{
build_from_non_nested(std::move(
cudf::test::fixed_width_column_wrapper<T, SourceElementT>(begin, end, v).release()));
build_from_non_nested(
cudf::test::fixed_width_column_wrapper<T, SourceElementT>(begin, end, v).release());
}

/**
Expand All @@ -1435,7 +1435,7 @@ class lists_column_wrapper : public detail::column_wrapper {
lists_column_wrapper(std::initializer_list<std::string> elements) : column_wrapper{}
{
build_from_non_nested(
std::move(cudf::test::strings_column_wrapper(elements.begin(), elements.end()).release()));
cudf::test::strings_column_wrapper(elements.begin(), elements.end()).release());
}

/**
Expand All @@ -1460,7 +1460,7 @@ class lists_column_wrapper : public detail::column_wrapper {
: column_wrapper{}
{
build_from_non_nested(
std::move(cudf::test::strings_column_wrapper(elements.begin(), elements.end(), v).release()));
cudf::test::strings_column_wrapper(elements.begin(), elements.end(), v).release());
}

/**
Expand Down
19 changes: 10 additions & 9 deletions cpp/src/groupby/hash/groupby.cu
Original file line number Diff line number Diff line change
Expand Up @@ -568,15 +568,16 @@ std::unique_ptr<table> groupby(table_view const& keys,
cudf::detail::result_cache sparse_results(requests.size());

auto const comparator_helper = [&](auto const d_key_equal) {
auto const set = cuco::static_set{num_keys,
0.5, // desired load factor
cuco::empty_key{cudf::detail::CUDF_SIZE_TYPE_SENTINEL},
d_key_equal,
probing_scheme_type{d_row_hash},
cuco::thread_scope_device,
cuco::storage<1>{},
cudf::detail::cuco_allocator{stream},
stream.value()};
auto const set = cuco::static_set{
num_keys,
0.5, // desired load factor
cuco::empty_key{cudf::detail::CUDF_SIZE_TYPE_SENTINEL},
d_key_equal,
probing_scheme_type{d_row_hash},
cuco::thread_scope_device,
cuco::storage<1>{},
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream},
stream.value()};

// Compute all single pass aggs first
compute_single_pass_aggs(keys,
Expand Down
35 changes: 18 additions & 17 deletions cpp/src/io/json/json_tree.cu
Original file line number Diff line number Diff line change
Expand Up @@ -545,15 +545,15 @@ rmm::device_uvector<size_type> hash_node_type_with_field_name(device_span<Symbol

using hasher_type = decltype(d_hasher);
constexpr size_type empty_node_index_sentinel = -1;
auto key_set =
cuco::static_set{cuco::extent{compute_hash_table_size(num_fields, 40)}, // 40% occupancy
cuco::empty_key{empty_node_index_sentinel},
d_equal,
cuco::linear_probing<1, hasher_type>{d_hasher},
{},
{},
cudf::detail::cuco_allocator{stream},
stream.value()};
auto key_set = cuco::static_set{
cuco::extent{compute_hash_table_size(num_fields, 40)}, // 40% occupancy
cuco::empty_key{empty_node_index_sentinel},
d_equal,
cuco::linear_probing<1, hasher_type>{d_hasher},
{},
{},
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream},
stream.value()};
key_set.insert_if_async(iter,
iter + num_nodes,
thrust::counting_iterator<size_type>(0), // stencil
Expand Down Expand Up @@ -734,14 +734,15 @@ std::pair<rmm::device_uvector<size_type>, rmm::device_uvector<size_type>> hash_n
constexpr size_type empty_node_index_sentinel = -1;
using hasher_type = decltype(d_hashed_cache);

auto key_set = cuco::static_set{cuco::extent{compute_hash_table_size(num_nodes)},
cuco::empty_key<cudf::size_type>{empty_node_index_sentinel},
d_equal,
cuco::linear_probing<1, hasher_type>{d_hashed_cache},
{},
{},
cudf::detail::cuco_allocator{stream},
stream.value()};
auto key_set = cuco::static_set{
cuco::extent{compute_hash_table_size(num_nodes)},
cuco::empty_key<cudf::size_type>{empty_node_index_sentinel},
d_equal,
cuco::linear_probing<1, hasher_type>{d_hashed_cache},
{},
{},
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream},
stream.value()};

// insert and convert node ids to unique set ids
auto nodes_itr = thrust::make_counting_iterator<size_type>(0);
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/io/json/write_json.cu
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ struct column_to_strings_fn {
auto const list_child_string = make_lists_column(
column.size(),
std::move(new_offsets),
std::move(child_string_with_null()),
child_string_with_null(),
column.null_count(),
cudf::detail::copy_bitmask(column, stream_, rmm::mr::get_current_device_resource()),
stream_);
Expand Down
52 changes: 26 additions & 26 deletions cpp/src/join/conditional_join.cu
Original file line number Diff line number Diff line change
Expand Up @@ -432,13 +432,13 @@ std::unique_ptr<rmm::device_uvector<size_type>> conditional_left_semi_join(
rmm::device_async_resource_ref mr)
{
CUDF_FUNC_RANGE();
return std::move(detail::conditional_join_anti_semi(left,
right,
binary_predicate,
detail::join_kind::LEFT_SEMI_JOIN,
output_size,
cudf::get_default_stream(),
mr));
return detail::conditional_join_anti_semi(left,
right,
binary_predicate,
detail::join_kind::LEFT_SEMI_JOIN,
output_size,
cudf::get_default_stream(),
mr);
}

std::unique_ptr<rmm::device_uvector<size_type>> conditional_left_anti_join(
Expand All @@ -449,13 +449,13 @@ std::unique_ptr<rmm::device_uvector<size_type>> conditional_left_anti_join(
rmm::device_async_resource_ref mr)
{
CUDF_FUNC_RANGE();
return std::move(detail::conditional_join_anti_semi(left,
right,
binary_predicate,
detail::join_kind::LEFT_ANTI_JOIN,
output_size,
cudf::get_default_stream(),
mr));
return detail::conditional_join_anti_semi(left,
right,
binary_predicate,
detail::join_kind::LEFT_ANTI_JOIN,
output_size,
cudf::get_default_stream(),
mr);
}

std::size_t conditional_inner_join_size(table_view const& left,
Expand Down Expand Up @@ -484,12 +484,12 @@ std::size_t conditional_left_semi_join_size(table_view const& left,
rmm::device_async_resource_ref mr)
{
CUDF_FUNC_RANGE();
return std::move(detail::compute_conditional_join_output_size(left,
right,
binary_predicate,
detail::join_kind::LEFT_SEMI_JOIN,
cudf::get_default_stream(),
mr));
return detail::compute_conditional_join_output_size(left,
right,
binary_predicate,
detail::join_kind::LEFT_SEMI_JOIN,
cudf::get_default_stream(),
mr);
}

std::size_t conditional_left_anti_join_size(table_view const& left,
Expand All @@ -498,12 +498,12 @@ std::size_t conditional_left_anti_join_size(table_view const& left,
rmm::device_async_resource_ref mr)
{
CUDF_FUNC_RANGE();
return std::move(detail::compute_conditional_join_output_size(left,
right,
binary_predicate,
detail::join_kind::LEFT_ANTI_JOIN,
cudf::get_default_stream(),
mr));
return detail::compute_conditional_join_output_size(left,
right,
binary_predicate,
detail::join_kind::LEFT_ANTI_JOIN,
cudf::get_default_stream(),
mr);
}

} // namespace cudf
2 changes: 1 addition & 1 deletion cpp/src/join/distinct_hash_join.cu
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ distinct_hash_join<HasNested>::distinct_hash_join(cudf::table_view const& build,
{},
cuco::thread_scope_device,
cuco_storage_type{},
cudf::detail::cuco_allocator{stream},
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream},
stream.value()}
{
CUDF_FUNC_RANGE();
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/join/hash_join.cu
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ hash_join<Hasher>::hash_join(cudf::table_view const& build,
cuco::empty_key{std::numeric_limits<hash_value_type>::max()},
cuco::empty_value{cudf::detail::JoinNoneValue},
stream.value(),
cudf::detail::cuco_allocator{stream}},
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream}},
_build{build},
_preprocessed_build{
cudf::experimental::row::equality::preprocessed_table::create(_build, stream)}
Expand Down
8 changes: 5 additions & 3 deletions cpp/src/join/join_common_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ using mixed_multimap_type =
cuco::static_multimap<hash_value_type,
size_type,
cuda::thread_scope_device,
cudf::detail::cuco_allocator,
cudf::detail::cuco_allocator<char>,
cuco::legacy::double_hashing<1, hash_type, hash_type>>;

using semi_map_type = cuco::legacy::
static_map<hash_value_type, size_type, cuda::thread_scope_device, cudf::detail::cuco_allocator>;
using semi_map_type = cuco::legacy::static_map<hash_value_type,
size_type,
cuda::thread_scope_device,
cudf::detail::cuco_allocator<char>>;

using row_hash_legacy =
cudf::row_hasher<cudf::hashing::detail::default_hash, cudf::nullate::DYNAMIC>;
Expand Down
22 changes: 12 additions & 10 deletions cpp/src/join/mixed_join.cu
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,12 @@ mixed_join(
auto build_view = table_device_view::create(build, stream);

// Don't use multimap_type because we want a CG size of 1.
mixed_multimap_type hash_table{compute_hash_table_size(build.num_rows()),
cuco::empty_key{std::numeric_limits<hash_value_type>::max()},
cuco::empty_value{cudf::detail::JoinNoneValue},
stream.value(),
cudf::detail::cuco_allocator{stream}};
mixed_multimap_type hash_table{
compute_hash_table_size(build.num_rows()),
cuco::empty_key{std::numeric_limits<hash_value_type>::max()},
cuco::empty_value{cudf::detail::JoinNoneValue},
stream.value(),
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream}};

// TODO: To add support for nested columns we will need to flatten in many
// places. However, this probably isn't worth adding any time soon since we
Expand Down Expand Up @@ -391,11 +392,12 @@ compute_mixed_join_output_size(table_view const& left_equality,
auto build_view = table_device_view::create(build, stream);

// Don't use multimap_type because we want a CG size of 1.
mixed_multimap_type hash_table{compute_hash_table_size(build.num_rows()),
cuco::empty_key{std::numeric_limits<hash_value_type>::max()},
cuco::empty_value{cudf::detail::JoinNoneValue},
stream.value(),
cudf::detail::cuco_allocator{stream}};
mixed_multimap_type hash_table{
compute_hash_table_size(build.num_rows()),
cuco::empty_key{std::numeric_limits<hash_value_type>::max()},
cuco::empty_value{cudf::detail::JoinNoneValue},
stream.value(),
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream}};

// TODO: To add support for nested columns we will need to flatten in many
// places. However, this probably isn't worth adding any time soon since we
Expand Down
11 changes: 6 additions & 5 deletions cpp/src/join/mixed_join_semi.cu
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,12 @@ std::unique_ptr<rmm::device_uvector<size_type>> mixed_join_semi(
cudf::experimental::row::equality::two_table_comparator{preprocessed_probe, preprocessed_build};
auto const equality_probe = row_comparator.equal_to<false>(has_nulls, compare_nulls);

semi_map_type hash_table{compute_hash_table_size(build.num_rows()),
cuco::empty_key{std::numeric_limits<hash_value_type>::max()},
cuco::empty_value{cudf::detail::JoinNoneValue},
cudf::detail::cuco_allocator{stream},
stream.value()};
semi_map_type hash_table{
compute_hash_table_size(build.num_rows()),
cuco::empty_key{std::numeric_limits<hash_value_type>::max()},
cuco::empty_value{cudf::detail::JoinNoneValue},
cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream},
stream.value()};

// Create hash table containing all keys found in right table
// TODO: To add support for nested columns we will need to flatten in many
Expand Down
12 changes: 7 additions & 5 deletions cpp/src/reductions/histogram.cu
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,13 @@ compute_row_frequencies(table_view const& input,
"Nested types are not yet supported in histogram aggregation.",
std::invalid_argument);

auto map = cudf::detail::hash_map_type{compute_hash_table_size(input.num_rows()),
cuco::empty_key{-1},
cuco::empty_value{std::numeric_limits<size_type>::min()},
cudf::detail::cuco_allocator{stream},
stream.value()};
auto map = cudf::detail::hash_map_type{
compute_hash_table_size(input.num_rows()),
cuco::empty_key{-1},
cuco::empty_value{std::numeric_limits<size_type>::min()},

cudf::detail::cuco_allocator<char>{rmm::mr::polymorphic_allocator<char>{}, stream},
stream.value()};

auto const preprocessed_input =
cudf::experimental::row::hash::preprocessed_table::create(input, stream);
Expand Down
Loading

0 comments on commit 8068a2d

Please sign in to comment.