Skip to content

Commit

Permalink
Move lists utility function definition out of header (#7266)
Browse files Browse the repository at this point in the history
Fixes #7265.

`cudf::detail::get_num_child_rows()` is currently defined in `cudf/lists/detail/utilities.cuh`. The build pipelines for #7189 are fine, but there seem to be build failures in dependent projects such as `spark-rapids`:
```
[2021-01-31T08:12:10.611Z] /.../workspace/spark/cudf18_nightly/cpp/include/cudf/lists/detail/utilities.cuh:31:18: error: 'cudf::size_type cudf::detail::get_num_child_rows(const cudf::column_view&, rmm::cuda_stream_view)' defined but not used [-Werror=unused-function]
[2021-01-31T08:12:10.611Z]  static cudf::size_type get_num_child_rows(cudf::column_view const& list_offsets,
[2021-01-31T08:12:10.611Z]                   ^~~~~~~~~~~~~~~~~~
[2021-01-31T08:12:11.981Z] cc1plus: all warnings being treated as errors
[2021-01-31T08:12:12.238Z] make[2]: *** [CMakeFiles/cudf_hash.dir/build.make:82: CMakeFiles/cudf_hash.dir/src/hash/hashing.cu.o] Error 1
[2021-01-31T08:12:12.238Z] make[1]: *** [CMakeFiles/Makefile2:220: CMakeFiles/cudf_hash.dir/all] Error 2
```
In any case, it is less than ideal for the function to be completely defined in the header, especially given that the likes of `hashing.cu` are exposed to it (by way of `scatter.cuh`). 

This commit moves the function definition to a separate translation unit, without changing implementation or interface.

Authors:
  - MithunR (@mythrocks)

Approvers:
  - @nvdbaranec
  - Mike Wilson (@hyperbolic2346)
  - David (@davidwendt)

URL: #7266
  • Loading branch information
mythrocks authored Feb 4, 2021
1 parent fd2d0e2 commit fd38b4c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 53 deletions.
14 changes: 9 additions & 5 deletions cpp/include/cudf/lists/detail/scatter.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
#include <cudf/column/column_device_view.cuh>
#include <cudf/column/column_factories.hpp>
#include <cudf/copying.hpp>
#include <cudf/detail/get_value.cuh>
#include <cudf/detail/valid_if.cuh>
#include <cudf/lists/detail/utilities.cuh>
#include <cudf/lists/list_device_view.cuh>
#include <cudf/null_mask.hpp>
#include <cudf/strings/detail/utilities.cuh>
Expand Down Expand Up @@ -333,7 +333,8 @@ struct list_child_constructor {
auto source_lists = cudf::detail::lists_column_device_view(*source_column_device_view);
auto target_lists = cudf::detail::lists_column_device_view(*target_column_device_view);

auto const num_child_rows{get_num_child_rows(list_offsets, stream)};
auto const num_child_rows{
cudf::detail::get_value<size_type>(list_offsets, list_offsets.size() - 1, stream)};

auto const child_null_mask =
source_lists_column_view.child().nullable() || target_lists_column_view.child().nullable()
Expand Down Expand Up @@ -427,7 +428,8 @@ struct list_child_constructor {
auto source_lists = cudf::detail::lists_column_device_view(*source_column_device_view);
auto target_lists = cudf::detail::lists_column_device_view(*target_column_device_view);

int32_t num_child_rows{get_num_child_rows(list_offsets, stream)};
auto const num_child_rows{
cudf::detail::get_value<size_type>(list_offsets, list_offsets.size() - 1, stream)};

auto string_views = rmm::device_vector<string_view>(num_child_rows);

Expand Down Expand Up @@ -516,7 +518,8 @@ struct list_child_constructor {
auto source_lists = cudf::detail::lists_column_device_view(*source_column_device_view);
auto target_lists = cudf::detail::lists_column_device_view(*target_column_device_view);

auto num_child_rows = get_num_child_rows(list_offsets, stream);
auto const num_child_rows{
cudf::detail::get_value<size_type>(list_offsets, list_offsets.size() - 1, stream)};

auto child_list_views = rmm::device_uvector<unbound_list_view>(num_child_rows, stream, mr);

Expand Down Expand Up @@ -621,7 +624,8 @@ struct list_child_constructor {
auto const source_structs = source_lists_column_view.child();
auto const target_structs = target_lists_column_view.child();

auto const num_child_rows = get_num_child_rows(list_offsets, stream);
auto const num_child_rows{
cudf::detail::get_value<size_type>(list_offsets, list_offsets.size() - 1, stream)};

auto const num_struct_members =
std::distance(source_structs.child_begin(), source_structs.child_end());
Expand Down
46 changes: 0 additions & 46 deletions cpp/include/cudf/lists/detail/utilities.cuh

This file was deleted.

5 changes: 3 additions & 2 deletions cpp/src/rolling/rolling_detail.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@
#include <cudf/detail/aggregation/aggregation.hpp>
#include <cudf/detail/copy.hpp>
#include <cudf/detail/gather.hpp>
#include <cudf/detail/get_value.cuh>
#include <cudf/detail/groupby/sort_helper.hpp>
#include <cudf/detail/nvtx/ranges.hpp>
#include <cudf/detail/utilities/cuda.cuh>
#include <cudf/detail/utilities/device_operators.cuh>
#include <cudf/detail/valid_if.cuh>
#include <cudf/dictionary/dictionary_column_view.hpp>
#include <cudf/dictionary/dictionary_factories.hpp>
#include <cudf/lists/detail/utilities.cuh>
#include <cudf/rolling.hpp>
#include <cudf/strings/detail/utilities.cuh>
#include <cudf/types.hpp>
Expand Down Expand Up @@ -983,7 +983,8 @@ struct rolling_window_launcher {
// This accounts for the `0` added by default to the offsets
// column, marking the beginning of the column.

auto num_child_rows = get_num_child_rows(offsets, stream);
auto const num_child_rows{
cudf::detail::get_value<size_type>(offsets, offsets.size() - 1, stream)};

auto scatter_values =
make_fixed_width_column(size_data_type, offsets.size(), mask_state::UNALLOCATED, stream, mr);
Expand Down

0 comments on commit fd38b4c

Please sign in to comment.