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.

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

New submdspan CPP 17 compatible #227

Merged
merged 27 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e004a42
Added new submdspan implementation
crtrott Dec 29, 2022
8cfc22f
Fix build errors on Mac
crtrott Dec 30, 2022
9c97013
Add missing strides parameter
crtrott Jan 3, 2023
4bf8a09
Account for strided_index_range stride in submdspan
crtrott Jan 4, 2023
4defa5e
Fix bug in submdspan_extents for strided_index_range slice
crtrott Jan 4, 2023
39450c1
Fix missing include
crtrott Jan 6, 2023
aefb6e5
Expand submdspan typed test to actually do runtime check
crtrott Jan 6, 2023
f516e85
Add simple strided_index_range test
crtrott Jan 6, 2023
ce9c272
Clang 14 needed explicit structured binding construction
crtrott Jan 6, 2023
f558537
Fix some comments and rename map to mapping in mapping_offset
crtrott Jan 6, 2023
8a4586b
Rewrite the logic to preserve layout_right and layout_left
crtrott Jan 6, 2023
79c1232
Add more tests
crtrott Jan 6, 2023
55c9979
Reformatting submdspan_extents
crtrott Jan 6, 2023
4749d80
Apply clang-format to submdspan files
crtrott Jan 6, 2023
c8e33dc
Backport new submdspan to C++17
crtrott Jan 6, 2023
8ff29f8
Make new submdspan work with NVCC and test on device
crtrott Jan 6, 2023
fa91897
Disable submdspan in C++14 mode
crtrott Jan 6, 2023
36b9376
Delete old submdspan impl
crtrott Jan 6, 2023
471ecbc
Update license files where missing
crtrott Jan 6, 2023
d7f2b3e
Based on Marks Review: always use C++17 inv_map_rank
crtrott Jan 6, 2023
a08fc05
Fix a comment
crtrott Jan 6, 2023
277bb26
Test submdspan customization point
crtrott Jan 7, 2023
7b80df4
Apply some changes suggested in review.
crtrott Jan 9, 2023
15899dc
Support and add test for compile time zero length/stride index range
crtrott Jan 9, 2023
98c6086
Fix some warnings and rename parameter pack
crtrott Jan 24, 2023
b4a7aeb
Actually run the submdspan test on device too
crtrott Jan 24, 2023
c99e6e5
Fix warnings
crtrott Jan 24, 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
6 changes: 5 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,11 @@ if(MDSPAN_ENABLE_EXAMPLES)
endif()

if(MDSPAN_ENABLE_BENCHMARKS)
add_subdirectory(benchmarks)
if(NOT MDSPAN_CXX_STANDARD STREQUAL "14")
add_subdirectory(benchmarks)
else()
MESSAGE(FATAL_ERROR "Benchmarks are not available in C++14 mode. Turn MDSPAN_ENABLE_BENCHMARKS OFF or use C++17 or newer.")
endif()
endif()

if(MDSPAN_ENABLE_COMP_BENCH)
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ When not in C++23 mode the implementation deviates from the proposal as follows:

### C++14
- deduction guides don't exist
- submdspan (P2630) is not available - an earlier variant of submdspan is available up to release 0.5 in C++14 mode
- benchmarks are not available (they need submdspan)



Expand Down
2 changes: 2 additions & 0 deletions compilation_tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@ if(NOT MDSPAN_ENABLE_CUDA)
add_compilation_test(ctest_compressed_pair_layout)
endif()
add_compilation_test(ctest_constexpr_dereference)
if(NOT CMAKE_CXX_STANDARD STREQUAL "14")
add_compilation_test(ctest_constexpr_submdspan)
endif()
add_compilation_test(ctest_constexpr_layouts)
558 changes: 0 additions & 558 deletions include/experimental/__p0009_bits/submdspan.hpp

This file was deleted.

40 changes: 40 additions & 0 deletions include/experimental/__p2630_bits/submdspan.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//@HEADER
// ************************************************************************
//
// Kokkos v. 4.0
// Copyright (2022) National Technology & Engineering
// Solutions of Sandia, LLC (NTESS).
//
// Under the terms of Contract DE-NA0003525 with NTESS,
// the U.S. Government retains certain rights in this software.
//
// Part of Kokkos, under the Apache License v2.0 with LLVM Exceptions.
// See https://kokkos.org/LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//@HEADER

#include "submdspan_extents.hpp"
#include "submdspan_mapping.hpp"

namespace std {
namespace experimental {
template <class ElementType, class Extents, class LayoutPolicy,
class AccessorPolicy, class... SliceSpecifiers>
MDSPAN_INLINE_FUNCTION
constexpr auto
submdspan(const mdspan<ElementType, Extents, LayoutPolicy, AccessorPolicy> &src,
SliceSpecifiers... slices) {
const auto sub_mapping_offset = submdspan_mapping(src.mapping(), slices...);
// NVCC has a problem with the deduction so lets figure out the type
using sub_mapping_t = std::remove_cv_t<decltype(sub_mapping_offset.mapping)>;
using sub_extents_t = typename sub_mapping_t::extents_type;
using sub_layout_t = typename sub_mapping_t::layout_type;
using sub_accessor_t = typename AccessorPolicy::offset_policy;
return mdspan<ElementType, sub_extents_t, sub_layout_t, sub_accessor_t>(
src.accessor().offset(src.data_handle(), sub_mapping_offset.offset),
sub_mapping_offset.mapping,
sub_accessor_t(src.accessor()));
}
} // namespace experimental
} // namespace std
315 changes: 315 additions & 0 deletions include/experimental/__p2630_bits/submdspan_extents.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,315 @@
//@HEADER
// ************************************************************************
//
// Kokkos v. 4.0
// Copyright (2022) National Technology & Engineering
// Solutions of Sandia, LLC (NTESS).
//
// Under the terms of Contract DE-NA0003525 with NTESS,
// the U.S. Government retains certain rights in this software.
//
// Part of Kokkos, under the Apache License v2.0 with LLVM Exceptions.
// See https://kokkos.org/LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//@HEADER

Copy link
Member

Choose a reason for hiding this comment

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

include headers as appropriate

namespace std {
namespace experimental {

// Slice Specifier allowing for strides and compile time extent
template <class OffsetType, class ExtentType, class StrideType>
struct strided_index_range {
using offset_type = OffsetType;
using extent_type = ExtentType;
using stride_type = StrideType;

OffsetType offset;
ExtentType extent;
StrideType stride;
};
dalg24 marked this conversation as resolved.
Show resolved Hide resolved

namespace detail {

// Mapping from submapping ranks to srcmapping ranks
// InvMapRank is an index_sequence, which we build recursively
// to contain the mapped indices.
// end of recursion specialization containing the final index_sequence
template <size_t Counter, size_t... MapIdxs>
MDSPAN_INLINE_FUNCTION
constexpr auto inv_map_rank(integral_constant<size_t, Counter>, index_sequence<MapIdxs...>) {
return index_sequence<MapIdxs...>();
};

// specialization reducing rank by one (i.e., integral slice specifier)
template<size_t Counter, class Slice, class... SliceSpecifiers, size_t... MapIdxs>
MDSPAN_INLINE_FUNCTION
constexpr auto inv_map_rank(integral_constant<size_t, Counter>, index_sequence<MapIdxs...>, Slice,
SliceSpecifiers... slices) {
using next_idx_seq_t = conditional_t<is_convertible_v<Slice, size_t>,
index_sequence<MapIdxs...>,
index_sequence<MapIdxs..., Counter>>;

return inv_map_rank(integral_constant<size_t,Counter + 1>(), next_idx_seq_t(),
slices...);
};

// Helper for identifying strided_index_range
template <class T> struct is_strided_index_range : false_type {};

template <class OffsetType, class ExtentType, class StrideType>
struct is_strided_index_range<
strided_index_range<OffsetType, ExtentType, StrideType>> : true_type {};

// first_of(slice): getting begin of slice specifier range
MDSPAN_TEMPLATE_REQUIRES(
class Integral,
/* requires */(is_convertible_v<Integral, size_t>)
)
MDSPAN_INLINE_FUNCTION
constexpr Integral first_of(const Integral &i) {
return i;
}

MDSPAN_INLINE_FUNCTION
constexpr integral_constant<size_t, 0>
first_of(const experimental::full_extent_t &) {
return integral_constant<size_t, 0>();
}

MDSPAN_TEMPLATE_REQUIRES(
class Slice,
/* requires */(is_convertible_v<Slice, tuple<size_t, size_t>>)
)
MDSPAN_INLINE_FUNCTION
constexpr auto first_of(const Slice &i) {
return get<0>(i);
}

template <class OffsetType, class ExtentType, class StrideType>
MDSPAN_INLINE_FUNCTION
constexpr OffsetType
first_of(const strided_index_range<OffsetType, ExtentType, StrideType> &r) {
return r.offset;
}

// last_of(slice): getting end of slice specifier range
dalg24 marked this conversation as resolved.
Show resolved Hide resolved
MDSPAN_TEMPLATE_REQUIRES(
size_t k, class Extents, class Integral,
/* requires */(is_convertible_v<Integral, size_t>)
)
MDSPAN_INLINE_FUNCTION
constexpr Integral
last_of(integral_constant<size_t, k>, const Extents &, const Integral &i) {
return i;
}

MDSPAN_TEMPLATE_REQUIRES(
size_t k, class Extents, class Slice,
/* requires */(is_convertible_v<Slice, tuple<size_t, size_t>>)
)
MDSPAN_INLINE_FUNCTION
constexpr auto last_of(integral_constant<size_t, k>, const Extents &,
const Slice &i) {
return get<1>(i);
}

// Suppress spurious warning with NVCC about no return statement.
Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen this with NVCC + GCC 10. The GCC intrinsic function __builtin_unreachable(); has helped me get rid of the warnings in this case. C++23 adds std::unreachable, which hopefully will accomplish the same goal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does that GCC intrinsic also work with nvc++, clang, msvc as host compiler?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a GCC-only thing, so when I used it, I defined it as a macro.

Copy link
Contributor

@mhoemmen mhoemmen Jan 6, 2023

Choose a reason for hiding this comment

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

GCC 10 isn't quite as smart as GCC 11 about constexpr auto functions that have exhaustive if constexpr branches in them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for figuring this out! : - )

// This is a known issue in NVCC and NVC++
#if defined __NVCC__
#ifdef __NVCC_DIAG_PRAGMA_SUPPORT__
#pragma nv_diagnostic push
#pragma nv_diag_suppress = implicit_return_from_non_void_function
#else
#pragma diagnostic push
#pragma diag_suppress = implicit_return_from_non_void_function
#endif
#elif defined __NVCOMPILER
#pragma diagnostic push
#pragma diag_suppress = implicit_return_from_non_void_function
#endif
template <size_t k, class Extents>
MDSPAN_INLINE_FUNCTION
constexpr auto last_of(integral_constant<size_t, k>, const Extents &ext,
experimental::full_extent_t) {
if constexpr (Extents::static_extent(k) == dynamic_extent) {
return ext.extent(k);
} else {
return integral_constant<size_t, Extents::static_extent(k)>();
}
}
#if defined __NVCC__
#ifdef __NVCC_DIAG_PRAGMA_SUPPORT__
#pragma nv_diagnostic pop
#else
#pragma diagnostic pop
#endif
#elif defined __NVCOMPILER
#pragma diagnostic pop
#endif

template <size_t k, class Extents, class OffsetType, class ExtentType,
class StrideType>
MDSPAN_INLINE_FUNCTION
constexpr OffsetType
last_of(integral_constant<size_t, k>, const Extents &,
const strided_index_range<OffsetType, ExtentType, StrideType> &r) {
return r.extent;
}

// get stride of slices
template <class T>
MDSPAN_INLINE_FUNCTION
constexpr auto stride_of(const T &) {
return integral_constant<size_t, 1>();
}

template <class OffsetType, class ExtentType, class StrideType>
MDSPAN_INLINE_FUNCTION
constexpr auto
stride_of(const strided_index_range<OffsetType, ExtentType, StrideType> &r) {
return r.stride;
}

// divide which can deal with integral constant preservation
template <class IndexT, class T0, class T1>
MDSPAN_INLINE_FUNCTION
constexpr auto divide(const T0 &v0, const T1 &v1) {
return IndexT(v0) / IndexT(v1);
}

template <class IndexT, class T0, T0 v0, class T1, T1 v1>
MDSPAN_INLINE_FUNCTION
constexpr auto divide(const integral_constant<T0, v0> &,
const integral_constant<T1, v1> &) {
return integral_constant<IndexT, v0 / v1>();
mhoemmen marked this conversation as resolved.
Show resolved Hide resolved
}

// multiply which can deal with integral constant preservation
template <class IndexT, class T0, class T1>
MDSPAN_INLINE_FUNCTION
constexpr auto multiply(const T0 &v0, const T1 &v1) {
return IndexT(v0) * IndexT(v1);
}

template <class IndexT, class T0, T0 v0, class T1, T1 v1>
MDSPAN_INLINE_FUNCTION
constexpr auto multiply(const integral_constant<T0, v0> &,
const integral_constant<T1, v1> &) {
return integral_constant<IndexT, v0 * v1>();
}

// compute new static extent from range, preserving static knowledge
template <class Arg0, class Arg1> struct StaticExtentFromRange {
constexpr static size_t value = dynamic_extent;
};

template <class Integral0, Integral0 val0, class Integral1, Integral1 val1>
struct StaticExtentFromRange<std::integral_constant<Integral0, val0>,
std::integral_constant<Integral1, val1>> {
constexpr static size_t value = val1 - val0;
};

// compute new static extent from strided_index_range, preserving static
// knowledge
template <class Arg0, class Arg1> struct StaticExtentFromStridedRange {
constexpr static size_t value = dynamic_extent;
};

template <class Integral0, Integral0 val0, class Integral1, Integral1 val1>
struct StaticExtentFromStridedRange<std::integral_constant<Integral0, val0>,
std::integral_constant<Integral1, val1>> {
constexpr static size_t value = val0 > 0 ? 1 + (val0 - 1) / val1 : 0;
};

// creates new extents through recursive calls to next_extent member function
// next_extent has different overloads for different types of stride specifiers
template <size_t K, class Extents, size_t... NewExtents>
struct extents_constructor {
MDSPAN_TEMPLATE_REQUIRES(
class Slice, class... SliceSpecifiers,
/* requires */(!is_convertible_v<Slice, size_t> &&
!is_strided_index_range<Slice>::value)
)
MDSPAN_INLINE_FUNCTION
constexpr static auto next_extent(const Extents &ext, const Slice &sl,
SliceSpecifiers... slices) {
Copy link
Contributor

Choose a reason for hiding this comment

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

More of a code readability issue -- the slices and new extent parameters here are kind of mixed up into the same parameter pack so it's difficult to understand the code. Might be worth separating out the new extents into a separate parameter.

constexpr size_t new_static_extent = StaticExtentFromRange<
mhoemmen marked this conversation as resolved.
Show resolved Hide resolved
decltype(first_of(std::declval<Slice>())),
decltype(last_of(integral_constant<size_t, Extents::rank() - K>(),
std::declval<Extents>(),
std::declval<Slice>()))>::value;

using next_t =
extents_constructor<K - 1, Extents, NewExtents..., new_static_extent>;
using index_t = typename Extents::index_type;
return next_t::next_extent(
ext, slices...,
index_t(last_of(integral_constant<size_t, Extents::rank() - K>(), ext,
sl)) -
index_t(first_of(sl)));
}

MDSPAN_TEMPLATE_REQUIRES(
class Slice, class... SliceSpecifiers,
/* requires */ (is_convertible_v<Slice, size_t>)
)
MDSPAN_INLINE_FUNCTION
constexpr static auto next_extent(const Extents &ext, const Slice &,
SliceSpecifiers... slices) {
using next_t = extents_constructor<K - 1, Extents, NewExtents...>;
return next_t::next_extent(ext, slices...);
}

template <class OffsetType, class ExtentType, class StrideType,
class... SliceSpecifiers>
MDSPAN_INLINE_FUNCTION
constexpr static auto
next_extent(const Extents &ext,
const strided_index_range<OffsetType, ExtentType, StrideType> &r,
SliceSpecifiers... slices) {
using index_t = typename Extents::index_type;
using new_static_extent_t =
StaticExtentFromStridedRange<ExtentType, StrideType>;
if constexpr (new_static_extent_t::value == dynamic_extent) {
using next_t =
extents_constructor<K - 1, Extents, NewExtents..., dynamic_extent>;
return next_t::next_extent(
ext, slices...,
r.extent > 0 ? 1 + divide<index_t>(r.extent - 1, r.stride) : 0);
Copy link
Contributor

@mhoemmen mhoemmen Jan 9, 2023

Choose a reason for hiding this comment

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

The code as it stands won't achieve the goal of divide, which is to return integral_constant if both inputs are integral_constant.

  1. Adding 1 to integral_constant<size_t, Value> results in size_t, not integral_constant<size_t, Value+1>.
  2. Even if adding 1 to integral_constant resulted in integral_constant, the result of the ternary operator would be common_type_t<integral_constant<size_t, Value>, int>, which is int.

https://godbolt.org/z/qWEdn5xM6

#include <utility>
#include <type_traits>

template<size_t Val1, size_t Val2>
constexpr auto divide(std::integral_constant<size_t, Val1> x,
  std::integral_constant<size_t, Val2> y) {
  return std::integral_constant<size_t, Val1 / Val2>{};
}

template<size_t Value>  
using Size = std::integral_constant<size_t, Value>;

int main() {
  constexpr auto z = divide(Size<8>{}, Size<2>{});
  static_assert(std::is_same_v<std::remove_const_t<decltype(z)>, Size<4>>);

  constexpr auto w = 1 + divide(Size<8>{}, Size<2>{});
  static_assert(not std::is_same_v<std::remove_const_t<decltype(w)>, Size<4>>);
  static_assert(std::is_same_v<std::remove_const_t<decltype(w)>, size_t>);

  static_assert(not std::is_same_v<std::common_type_t<Size<4>, int>, Size<4>>);
  static_assert(not std::is_same_v<std::common_type_t<Size<4>, int>, size_t>);
  static_assert(std::is_same_v<std::common_type_t<Size<4>, int>, int>);

  return 0;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't matter since it doesn't determine the type. Note the values I am generating here are used as constructor arguments for an extents type which is independently determined. In case static extents are generated or preserved, these values will at best be checked against the static_extents in the extents constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

integral_constant<T, Value> implicitly converts to T. This means that 1 + integral_constant<size_t, X>{} / integral_constant<size_t, Y>{} just returns 1 + X / Y, so you don't even need divide here.

https://godbolt.org/z/x48bnaW8e

} else {
constexpr size_t new_static_extent = new_static_extent_t::value;
using next_t =
extents_constructor<K - 1, Extents, NewExtents..., new_static_extent>;
return next_t::next_extent(
ext, slices..., index_t(divide<index_t>(ExtentType(), StrideType())));
}
}
};

template <class Extents, size_t... NewStaticExtents>
struct extents_constructor<0, Extents, NewStaticExtents...> {

template <class... NewExtents>
MDSPAN_INLINE_FUNCTION
constexpr static auto next_extent(const Extents &, NewExtents... new_exts) {
return extents<typename Extents::index_type, NewStaticExtents...>(
new_exts...);
}
};

} // namespace detail

// submdspan_extents creates new extents given src extents and submdspan slice
// specifiers
template <class IndexType, size_t... Extents, class... SliceSpecifiers>
MDSPAN_INLINE_FUNCTION
constexpr auto submdspan_extents(const extents<IndexType, Extents...> &src_exts,
SliceSpecifiers... slices) {

using ext_t = extents<IndexType, Extents...>;
return detail::extents_constructor<ext_t::rank(), ext_t>::next_extent(
src_exts, slices...);
}
} // namespace experimental
} // namespace std
Loading