Skip to content
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.

Sign up for GitHub

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

Support gcc 12 as the C++ compiler #13316

Merged
merged 6 commits into from
May 11, 2023
Merged

Support gcc 12 as the C++ compiler #13316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b69dbfb
Fix compile issues found with nvcc 12.0 + gcc 12.2
robertmaynard Apr 26, 2023
7c59da0
nulls_from_nullptrs now captures ptr by reference instead of by value.
robertmaynard May 8, 2023
ed47700
Use the explicit conversion operator on fixed_point classes
robertmaynard May 9, 2023
f452e50
Correct GCC 12 stringop-overflow= warnings with initializer_list
robertmaynard May 9, 2023
7190069
Ensure all rapids-cmake nvbench patches are applied
robertmaynard May 9, 2023
2488a84
Remove unneeded capturing
robertmaynard May 9, 2023
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
5 changes: 5 additions & 0 deletions cpp/cmake/thirdparty/patches/nvbench_override.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
"file" : "nvbench/use_existing_fmt.diff",
"issue" : "Fix add support for using an existing fmt [https://github.com/NVIDIA/nvbench/pull/125]",
"fixed_in" : ""
},
{
"file" : "nvbench/public_fmt_dep_in_conda.diff",
"issue" : "Propagate fmt requirement in conda envs [https://github.com/NVIDIA/nvbench/pull/127]",
"fixed_in" : ""
}
]
}
Expand Down
8 changes: 5 additions & 3 deletions cpp/include/cudf_test/iterator_utilities.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
* Copyright (c) 2021-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -121,15 +121,17 @@ template <typename Iter>
* The returned iterator yields `false` (to mark `null`) at the indices corresponding to the
* pointers having `nullptr` values and `true` for the remaining indices.
*
* @note The input vector is referenced by the transform iterator, so the
* lifespan must be just as long as the iterator.
*
* @tparam T the data type
* @param ptrs The data pointers for which the validity iterator is computed
* @return auto Validity iterator
*/
template <class T>
[[maybe_unused]] static auto nulls_from_nullptrs(std::vector<T const*> const& ptrs)
{
// The vector `indices` is copied into the lambda as it can be destroyed at the caller site.
return thrust::make_transform_iterator(ptrs.begin(), [ptrs](auto ptr) { return ptr != nullptr; });
return thrust::make_transform_iterator(ptrs.begin(), [](auto ptr) { return ptr != nullptr; });
}

} // namespace iterators
Expand Down
14 changes: 8 additions & 6 deletions cpp/include/cudf_test/type_lists.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
* Copyright (c) 2019-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -84,11 +84,13 @@ std::enable_if_t<cudf::is_fixed_width<TypeParam>() && !cudf::is_timestamp_t<Type
thrust::host_vector<TypeParam>>
make_type_param_vector(std::initializer_list<T> const& init_list)
{
thrust::host_vector<TypeParam> vec(init_list.size());
std::transform(std::cbegin(init_list), std::cend(init_list), std::begin(vec), [](auto const& e) {
if constexpr (std::is_unsigned_v<TypeParam>) { return static_cast<TypeParam>(std::abs(e)); }
return static_cast<TypeParam>(e);
});
std::vector<T> input{init_list};
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to copy the init_list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reproducer:

#include <algorithm>
#include <cstdint>
#include <vector>
#include <thrust/host_vector.h>


template <typename TypeParam, typename T>
thrust::host_vector<TypeParam>
make_type_param_vector(std::initializer_list<T> const& init_list) {
  // std::vector<T> input{init_list};
  std::vector<TypeParam> vec(init_list.size()); //or thrust::host_vector
  std::transform(std::cbegin(init_list), std::cend(init_list), std::begin(vec), [](auto const& e) {
    if constexpr (std::is_unsigned_v<TypeParam>) { return static_cast<TypeParam>(std::abs(e)); }
    return static_cast<TypeParam>(e);
  });
  return vec;
}


template <typename T> bool validate_A() {
  auto const input_column_valid_a = make_type_param_vector<uint8_t>({1, 0});
  auto const input_column_valid_b = make_type_param_vector<uint8_t>({0, 0});
  auto const input_column_valid_c = make_type_param_vector<T>({15, 16});
  return 0;
}
int main() {
  validate_A<float>();
  validate_A<double>();
  validate_A<int8_t>();
  validate_A<int16_t>();
  validate_A<int32_t>();
  validate_A<int64_t>();
  validate_A<uint8_t>();
  validate_A<uint16_t>();
  validate_A<uint32_t>();
  return 0;
}

Will generate warnings ( as errors ) of:

    inlined from 'thrust::host_vector<TypeParam> make_type_param_vector(const std::initializer_list<_Value>&) [with TypeParam = unsigned char; T = int]' at /home/rmaynard/Work/temp/sample2.cpp:11:34:
/home/rmaynard/miniconda3/envs/cuda_tk12/x86_64-conda-linux-gnu/include/c++/12.2.0/bits/new_allocator.h:137:55: note: at offset [29, 4611686018427387917] into destination object of size 3 allocated by 'operator new'
  137 |         return static_cast<_Tp*>(_GLIBCXX_OPERATOR_NEW(__n * sizeof(_Tp)));
      |                                                       ^
In function '_OIter std::transform(_IIter, _IIter, _OIter, _UnaryOperation) [with _IIter = const int*; _OIter = thrust::detail::normal_iterator<unsigned char*>; _UnaryOperation = make_type_param_vector<unsigned char, int>(const std::initializer_list<int>&)::<lambda(const auto:1&)>]',
    inlined from 'thrust::host_vector<TypeParam> make_type_param_vector(const std::initializer_list<_Value>&) [with TypeParam = unsigned char; T = int]' at /home/rmaynard/Work/temp/sample2.cpp:12:17:
/home/rmaynard/miniconda3/envs/cuda_tk12/x86_64-conda-linux-gnu/include/c++/12.2.0/bits/stl_algo.h:4263:19: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 4263 |         *__result = __unary_op(*__first);

I believe that this is a bug in gcc as it is easit to trigger when we have at least 3 calls to make_type_param_vector with input of the same length with no other calls of a differing length to make_type_param_vector. Copying the initializer_list to a std::vector avoids this bug.

std::vector<TypeParam> vec(init_list.size());
std::transform(
std::cbegin(input), std::cend(input), std::begin(vec), [](auto const& e) -> TypeParam {
if constexpr (std::is_unsigned_v<TypeParam>) { return static_cast<TypeParam>(std::abs(e)); }
return static_cast<TypeParam>(e);
});
return vec;
}

Expand Down
6 changes: 3 additions & 3 deletions cpp/src/io/json/json_column.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,10 +813,10 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> device_json_co

// For string columns return ["offsets", "char"] schema
if (target_type.id() == type_id::STRING) {
return {std::move(col), {{"offsets"}, {"chars"}}};
return {std::move(col), std::vector<column_name_info>{{"offsets"}, {"chars"}}};
}
// Non-string leaf-columns (e.g., numeric) do not have child columns in the schema
return {std::move(col), {}};
return {std::move(col), std::vector<column_name_info>{}};
}
case json_col_t::StructColumn: {
std::vector<std::unique_ptr<column>> child_columns;
Expand Down Expand Up @@ -860,7 +860,7 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> device_json_co
data_type{type_id::INT8},
0,
rmm::device_buffer{0, stream, mr}),
{}}
std::vector<column_name_info>{}}
: device_json_column_to_cudf_column(
json_col.child_columns.begin()->second,
d_input,
Expand Down
7 changes: 4 additions & 3 deletions cpp/src/io/json/nested_json_gpu.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1678,11 +1678,11 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> json_column_to

// For string columns return ["offsets", "char"] schema
if (target_type.id() == type_id::STRING) {
return {std::move(col), {{"offsets"}, {"chars"}}};
return {std::move(col), std::vector<column_name_info>{{"offsets"}, {"chars"}}};
}
// Non-string leaf-columns (e.g., numeric) do not have child columns in the schema
else {
return {std::move(col), {}};
return {std::move(col), std::vector<column_name_info>{}};
}
break;
}
Expand Down Expand Up @@ -1724,7 +1724,8 @@ std::pair<std::unique_ptr<column>, std::vector<column_name_info>> json_column_to
auto [child_column, names] =
json_col.child_columns.empty()
? std::pair<std::unique_ptr<column>,
std::vector<column_name_info>>{std::make_unique<column>(), {}}
std::vector<column_name_info>>{std::make_unique<column>(),
std::vector<column_name_info>{}}
: json_column_to_cudf_column(json_col.child_columns.begin()->second,
d_input,
options,
Expand Down
2 changes: 1 addition & 1 deletion cpp/tests/io/json_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1568,7 +1568,7 @@ TEST_P(JsonReaderParamTest, JsonDtypeParsing)

auto make_validity = [](std::vector<int> const& validity) {
return cudf::detail::make_counting_transform_iterator(
0, [=](auto i) -> bool { return static_cast<bool>(validity[i]); });
0, [&](auto i) -> bool { return static_cast<bool>(validity[i]); });
};

constexpr int int_ignore{};
Expand Down
4 changes: 2 additions & 2 deletions cpp/tests/scalar/scalar_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ TYPED_TEST_SUITE(TypedScalarTestWithoutFixedPoint, cudf::test::FixedWidthTypesWi
TYPED_TEST(TypedScalarTest, DefaultValidity)
{
using Type = cudf::device_storage_type_t<TypeParam>;
Type value = cudf::test::make_type_param_scalar<TypeParam>(7);
Type value = static_cast<Type>(cudf::test::make_type_param_scalar<TypeParam>(7));
cudf::scalar_type_t<TypeParam> s(value);

EXPECT_TRUE(s.is_valid());
Expand Down Expand Up @@ -71,7 +71,7 @@ TYPED_TEST(TypedScalarTestWithoutFixedPoint, SetNull)
TYPED_TEST(TypedScalarTest, CopyConstructor)
{
using Type = cudf::device_storage_type_t<TypeParam>;
Type value = cudf::test::make_type_param_scalar<TypeParam>(8);
Type value = static_cast<Type>(cudf::test::make_type_param_scalar<TypeParam>(8));
cudf::scalar_type_t<TypeParam> s(value);
auto s2 = s;

Expand Down