Skip to content

[Mirror] Implement canonicalize slices for mdspan (P3663) #18

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

Open
csiefer2 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

[Mirror] Implement canonicalize slices for mdspan (P3663) #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1975b77
Implement canonicalize slices for mdspan (P3663)
crtrott Jun 26, 2026
a062b37
Reimport layout_left/right_padded to experimental
crtrott Jun 26, 2026
3790bf4
Fix padded layouts required_span_size
crtrott Jun 26, 2026
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
7 changes: 7 additions & 0 deletions core/src/View/MDSpan/Kokkos_MDSpan_Layout.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ static_assert(false,
// mdspan layout is that the array layouts can have state, but don't have the
// nested mapping. This file provides interoperability helpers.

namespace Kokkos::Experimental {
template <size_t Pad = Kokkos::dynamic_extent>
using layout_left_padded = Kokkos::layout_left_padded<Pad>;
template <size_t Pad = Kokkos::dynamic_extent>
using layout_right_padded = Kokkos::layout_right_padded<Pad>;
} // namespace Kokkos::Experimental

namespace Kokkos::Impl {
// We do have implementation detail versions of these in our mdspan impl
// However they are not part of the public standard interface
Expand Down
6 changes: 6 additions & 0 deletions tpls/mdspan/include/experimental/__p0009_bits/config.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@
#else
#define MDSPAN_CXX_STD_23 202100L
#endif
#define MDSPAN_CXX_STD_26 202603L

#define MDSPAN_HAS_CXX_14 (MDSPAN_IMPL_CPLUSPLUS >= MDSPAN_CXX_STD_14)
#define MDSPAN_HAS_CXX_17 (MDSPAN_IMPL_CPLUSPLUS >= MDSPAN_CXX_STD_17)
#define MDSPAN_HAS_CXX_20 (MDSPAN_IMPL_CPLUSPLUS >= MDSPAN_CXX_STD_20)
#define MDSPAN_HAS_CXX_23 (MDSPAN_IMPL_CPLUSPLUS >= MDSPAN_CXX_STD_23)
#define MDSPAN_HAS_CXX_26 (MDSPAN_IMPL_CPLUSPLUS >= MDSPAN_CXX_STD_26)

static_assert(MDSPAN_IMPL_CPLUSPLUS >= MDSPAN_CXX_STD_14, "mdspan requires C++14 or later.");

Expand Down Expand Up @@ -303,3 +305,7 @@ static_assert(MDSPAN_IMPL_CPLUSPLUS >= MDSPAN_CXX_STD_14, "mdspan requires C++14
# define MDSPAN_IMPL_OP5(mds, a, b, c, d, e) mds(a,b,c,d,e)
# define MDSPAN_IMPL_OP6(mds, a, b, c, d, e, f) mds(a,b,c,d,e,f)
#endif

#if ! defined(MDSPAN_IMPL_ENABLE_P3663)
# define MDSPAN_IMPL_ENABLE_P3663 1
#endif
31 changes: 26 additions & 5 deletions tpls/mdspan/include/experimental/__p0009_bits/extents.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,12 @@ template <class IndexType, size_t... Extents> class extents {
sizeof...(OtherIndexTypes) == m_rank_dynamic)))
MDSPAN_INLINE_FUNCTION
constexpr explicit extents(OtherIndexTypes... dynvals) noexcept
: m_vals(static_cast<index_type>(dynvals)...) {}
: m_vals(static_cast<index_type>(dynvals)...) {
#if MDSPAN_HAS_CXX_17
MDSPAN_IMPL_PRECONDITION(
detail::all_values_are_nonnegative_and_representable<index_type>(dynvals...));
#endif
}

MDSPAN_TEMPLATE_REQUIRES(
class OtherIndexType, size_t N,
Expand All @@ -452,7 +457,13 @@ template <class IndexType, size_t... Extents> class extents {
MDSPAN_INLINE_FUNCTION
MDSPAN_CONDITIONAL_EXPLICIT(N != m_rank_dynamic)
constexpr extents(const std::array<OtherIndexType, N> &exts) noexcept
: m_vals(std::move(exts)) {}
: m_vals(std::move(exts)) {
#if MDSPAN_HAS_CXX_17
MDSPAN_IMPL_PRECONDITION(
detail::range_is_nonnegative_and_representable<index_type>(
std::begin(exts), std::end(exts)));
#endif
}

#ifdef __cpp_lib_span
MDSPAN_TEMPLATE_REQUIRES(
Expand All @@ -464,7 +475,11 @@ template <class IndexType, size_t... Extents> class extents {
MDSPAN_INLINE_FUNCTION
MDSPAN_CONDITIONAL_EXPLICIT(N != m_rank_dynamic)
constexpr extents(const std::span<OtherIndexType, N> &exts) noexcept
: m_vals(std::move(exts)) {}
: m_vals(std::move(exts)) {
MDSPAN_IMPL_PRECONDITION(
detail::range_is_nonnegative_and_representable<index_type>(
std::begin(exts), std::end(exts)));
}
#endif

private:
Expand Down Expand Up @@ -536,10 +551,16 @@ template <class IndexType, size_t... Extents> class extents {
...) ||
(std::numeric_limits<index_type>::max() <
std::numeric_limits<OtherIndexType>::max()))
constexpr extents(const extents<OtherIndexType, OtherExtents...> &other) noexcept
constexpr extents(
const extents<OtherIndexType, OtherExtents...> &other) noexcept
: m_vals(impl_construct_vals_from_extents(
std::integral_constant<size_t, 0>(),
std::integral_constant<size_t, 0>(), other)) {}
std::integral_constant<size_t, 0>(), other)) {
#if MDSPAN_HAS_CXX_17
MDSPAN_IMPL_PRECONDITION(
detail::extent_is_representable<index_type>(other));
#endif
}

// Comparison operator
template <class OtherIndexType, size_t... OtherExtents>
Expand Down
40 changes: 19 additions & 21 deletions tpls/mdspan/include/experimental/__p0009_bits/layout_left.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,30 +118,28 @@ class layout_left::mapping {
/**
* Converting constructor from `layout_left_padded::mapping`.
*
* This overload participates in overload resolution only if Mapping is a layout_left_padded mapping and
* extents_type is constructible from Mapping::extents_type.
* This overload participates in overload resolution only if Mapping is a
* layout_left_padded mapping and extents_type is constructible from
* Mapping::extents_type.
*
* \note There is currently a difference from p2642r2, where this function is specified as taking
* `layout_left_padded< padding_value >::mapping< Extents>`. However, this makes `padding_value` non-deducible.
* \note There is currently a difference from p2642r2, where this function
* is specified as taking `layout_left_padded< padding_value >::mapping<
* Extents>`. However, this makes `padding_value` non-deducible.
*/
MDSPAN_TEMPLATE_REQUIRES(
class Mapping,
/* requires */ (
MDSPAN_IMPL_PROPOSED_NAMESPACE::detail::is_layout_left_padded_mapping<Mapping>::value
&& std::is_constructible_v<extents_type, typename Mapping::extents_type>
)
)
MDSPAN_CONDITIONAL_EXPLICIT((!std::is_convertible_v<typename Mapping::extents_type, extents_type>))
MDSPAN_INLINE_FUNCTION constexpr
mapping(const Mapping& other) noexcept
: m_extents(other.extents())
{
MDSPAN_IMPL_PROPOSED_NAMESPACE::detail::
check_padded_layout_converting_constructor_mandates<
extents_type, Mapping>(detail::with_rank<extents_type::rank()>{});
MDSPAN_IMPL_PROPOSED_NAMESPACE::detail::
check_padded_layout_converting_constructor_preconditions<
extents_type>(detail::with_rank<extents_type::rank()>{}, other);
class Mapping,
/* requires */ (detail::is_layout_left_padded_mapping<Mapping>::value
&&std::is_constructible_v<
extents_type, typename Mapping::extents_type>))
MDSPAN_CONDITIONAL_EXPLICIT(
(!std::is_convertible_v<typename Mapping::extents_type, extents_type>))
MDSPAN_INLINE_FUNCTION constexpr mapping(const Mapping &other) noexcept
: m_extents(other.extents()) {
detail::check_padded_layout_converting_constructor_mandates<extents_type,
Mapping>(
detail::with_rank<extents_type::rank()>{});
detail::check_padded_layout_converting_constructor_preconditions<
extents_type>(detail::with_rank<extents_type::rank()>{}, other);
}
#endif

Expand Down
26 changes: 12 additions & 14 deletions tpls/mdspan/include/experimental/__p0009_bits/layout_right.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,20 +128,18 @@ class layout_right::mapping {
#if MDSPAN_HAS_CXX_17
MDSPAN_TEMPLATE_REQUIRES(
class Mapping,
/* requires */ (
MDSPAN_IMPL_PROPOSED_NAMESPACE::detail::is_layout_right_padded_mapping<Mapping>::value
&& std::is_constructible_v<extents_type, typename Mapping::extents_type>))
MDSPAN_CONDITIONAL_EXPLICIT((!std::is_convertible_v<typename Mapping::extents_type, extents_type>))
MDSPAN_INLINE_FUNCTION constexpr
mapping(const Mapping &other) noexcept
: m_extents(other.extents())
{
MDSPAN_IMPL_PROPOSED_NAMESPACE::detail::
check_padded_layout_converting_constructor_mandates<
extents_type, Mapping>(detail::with_rank<extents_type::rank()>{});
MDSPAN_IMPL_PROPOSED_NAMESPACE::detail::
check_padded_layout_converting_constructor_preconditions<
extents_type>(detail::with_rank<extents_type::rank()>{}, other);
/* requires */ (detail::is_layout_right_padded_mapping<Mapping>::value
&&std::is_constructible_v<
extents_type, typename Mapping::extents_type>))
MDSPAN_CONDITIONAL_EXPLICIT(
(!std::is_convertible_v<typename Mapping::extents_type, extents_type>))
MDSPAN_INLINE_FUNCTION constexpr mapping(const Mapping &other) noexcept
: m_extents(other.extents()) {
detail::check_padded_layout_converting_constructor_mandates<extents_type,
Mapping>(
detail::with_rank<extents_type::rank()>{});
detail::check_padded_layout_converting_constructor_preconditions<
extents_type>(detail::with_rank<extents_type::rank()>{}, other);
}
#endif

Expand Down
6 changes: 6 additions & 0 deletions tpls/mdspan/include/experimental/__p0009_bits/layout_stride.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
#include "compressed_pair.hpp"
#include "utility.hpp"

#if MDSPAN_HAS_CXX_17
#include "../__p2642_bits/layout_padded_fwd.hpp"
#endif
Comment on lines +24 to +26

@gemini-code-assist gemini-code-assist Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In C++14, layout_padded_fwd.hpp is not included because MDSPAN_HAS_CXX_17 is false. However, detail::is_layout_left_padded_mapping and detail::is_layout_right_padded_mapping are still referenced in MDSPAN_CONDITIONAL_EXPLICIT (which is compiled in C++14), leading to compilation failures. We can define helper traits that fall back to std::false_type in C++14 to maintain compatibility.

#if MDSPAN_HAS_CXX_17
#include "../__p2642_bits/layout_padded_fwd.hpp"
namespace MDSPAN_IMPL_STANDARD_NAMESPACE {
namespace detail {
template <class Mapping>
using is_layout_left_padded_mapping_or_false = is_layout_left_padded_mapping<Mapping>;
template <class Mapping>
using is_layout_right_padded_mapping_or_false = is_layout_right_padded_mapping<Mapping>;
}
}
#else
namespace MDSPAN_IMPL_STANDARD_NAMESPACE {
namespace detail {
template <class Mapping>
struct is_layout_left_padded_mapping_or_false : std::false_type {};
template <class Mapping>
struct is_layout_right_padded_mapping_or_false : std::false_type {};
}
}
#endif


#if !defined(MDSPAN_IMPL_USE_ATTRIBUTE_NO_UNIQUE_ADDRESS)
# include "no_unique_address.hpp"
#endif
Expand Down Expand Up @@ -441,6 +445,8 @@ struct layout_stride {
!(std::is_convertible<typename StridedLayoutMapping::extents_type, extents_type>::value &&
(detail::is_mapping_of<layout_left, StridedLayoutMapping> ||
detail::is_mapping_of<layout_right, StridedLayoutMapping> ||
detail::is_layout_left_padded_mapping<StridedLayoutMapping>::value || // Don't need to guard for C++14 as this isn't compiled in < C++20
detail::is_layout_right_padded_mapping<StridedLayoutMapping>::value ||
Comment on lines +448 to +449

@gemini-code-assist gemini-code-assist Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Use the C++14-compatible helper traits to avoid compilation errors when compiling in C++14 mode.

        detail::is_layout_left_padded_mapping_or_false<StridedLayoutMapping>::value ||
        detail::is_layout_right_padded_mapping_or_false<StridedLayoutMapping>::value ||

detail::is_mapping_of<layout_stride, StridedLayoutMapping>))
) // needs two () due to comma
MDSPAN_INLINE_FUNCTION MDSPAN_IMPL_CONSTEXPR_14
Expand Down
69 changes: 69 additions & 0 deletions tpls/mdspan/include/experimental/__p0009_bits/utility.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@
namespace MDSPAN_IMPL_STANDARD_NAMESPACE {
namespace detail {

// Backport of std::remove_cvref / std::remove_cvref_t (C++20)
#if (__cplusplus >= 202002L)
using std::remove_cvref_t;
#else
template<class T>
using remove_cvref_t = std::remove_cv_t<std::remove_reference_t<T>>;
#endif // __cplusplus >= 202002L

// type alias used for rank-based tag dispatch
//
// this is used to enable alternatives to constexpr if when building for C++14
Expand Down Expand Up @@ -203,6 +211,8 @@ MDSPAN_INLINE_FUNCTION constexpr bool cmp_greater_equal(T t, U u) noexcept {

template <class R, class T>
MDSPAN_INLINE_FUNCTION constexpr bool in_range(T t) noexcept {
static_assert(std::is_integral_v<R> && std::is_integral_v<T>);

#if defined(MDSPAN_IMPL_HAS_CUDA) && defined(__NVCC__) && (__CUDACC_VER_MAJOR__ * 100 + __CUDACC_VER_MINOR__ * 10 >= 1260)
using cuda::std::numeric_limits;
#else
Expand All @@ -212,6 +222,65 @@ MDSPAN_INLINE_FUNCTION constexpr bool in_range(T t) noexcept {
cmp_less_equal(t, numeric_limits<R>::max());
}

template <class R, class T>
MDSPAN_INLINE_FUNCTION constexpr bool is_nonnegative_and_representable(T t) noexcept {
// T might not be integral and thus invalid to pass to in_range
// Only check this if we can actually call in_range
if constexpr (std::is_integral_v<T>)
{
if constexpr (std::is_signed_v<T>) {
if (t < 0)
return false;
}

return in_range<R>(t);
} else
{
if constexpr (std::is_signed_v<R>) {
if (static_cast<R>(t) < 0)
return false;
}

return true;
}
}
Comment on lines +225 to +246

@gemini-code-assist gemini-code-assist Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In is_nonnegative_and_representable, if T is an integral-constant-like type (such as std::integral_constant or constant_wrapper) wrapping a negative value, std::is_integral_v<T> evaluates to false. If R is an unsigned type (like size_t), the else branch will be taken, and since std::is_signed_v<R> is false, the function will incorrectly return true for negative values. This bypasses precondition checks and can lead to silent undefined behavior.

To fix this, we can define a helper to extract the underlying value of integral-constant-like types before performing the checks.

template <class T, class = void>
struct has_value_member : std::false_type {};

template <class T>
struct has_value_member<T, decltype((void)T::value, void())> : std::true_type {};

template <class T>
constexpr auto get_absolute_value(T t) {
  if constexpr (has_value_member<T>::value) {
    return T::value;
  } else {
    return t;
  }
}

template <class R, class T>
MDSPAN_INLINE_FUNCTION constexpr bool is_nonnegative_and_representable(T t) noexcept {
  auto val = get_absolute_value(t);
  using V = decltype(val);
  if constexpr (std::is_integral_v<V>)
  {
    if constexpr (std::is_signed_v<V>) {
      if (val < 0)
        return false;
    }

    return in_range<R>(val);
  } else
  {
    if constexpr (std::is_signed_v<R>) {
      if (static_cast<R>(val) < 0)
        return false;
    }

    return true;
  }
}


template<class R, class... Values>
MDSPAN_INLINE_FUNCTION constexpr bool
all_values_are_representable(Values... values) noexcept {
return ( in_range<R>( values ) && ... );
}

template<class R, class... Values>
MDSPAN_INLINE_FUNCTION constexpr bool
all_values_are_nonnegative_and_representable(Values... values) noexcept {
return ( is_nonnegative_and_representable<R>( values ) && ... );
}

template<class R, class ContiguousIterator>
MDSPAN_INLINE_FUNCTION constexpr bool
range_is_nonnegative_and_representable(ContiguousIterator begin, ContiguousIterator end) noexcept {
for ( auto it = begin; it < end; ++it )
{
if ( !is_nonnegative_and_representable<R>( *it ) )
return false;
}

return true;
}

template<class R, class Extents>
MDSPAN_INLINE_FUNCTION constexpr bool
extent_is_representable(const Extents &exts) noexcept {
for ( std::size_t r = 0; r < Extents::rank(); ++r )
{
if ( !is_nonnegative_and_representable<R>( exts.extent(r) ) )
return false;
}

return true;
}

template <typename T >
MDSPAN_INLINE_FUNCTION constexpr bool
check_mul_result_is_nonnegative_and_representable(T a, T b) {
Expand Down
3 changes: 0 additions & 3 deletions tpls/mdspan/include/experimental/__p2389_bits/dims.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@

#pragma once

// backward compatibility import into experimental
namespace MDSPAN_IMPL_STANDARD_NAMESPACE {
namespace MDSPAN_IMPL_PROPOSED_NAMESPACE {

template< ::std::size_t Rank, class IndexType = std::size_t>
using dims =
:: MDSPAN_IMPL_STANDARD_NAMESPACE :: dextents<IndexType, Rank>;

} // namespace MDSPAN_IMPL_PROPOSED_NAMESPACE
} // namespace MDSPAN_IMPL_STANDARD_NAMESPACE
Loading
Loading