Skip to content

precondition checks in extents constructors #410

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

Merged
nmm0 merged 8 commits into from
Feb 16, 2026
+166 −6
Merged

precondition checks in extents constructors #410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
89afaea
#409: add precondition checks in extents constructors
nmm0 Jun 2, 2025
ef12e3e
#409: only check extents preconditions in C++17 and above mode
nmm0 Jun 16, 2025
11d9a77
#409: disable precondition checks for extent in C++ < 17
nmm0 Jun 16, 2025
8667593
#409: only check in_range for integral extents arguments; additionall…
nmm0 Jun 16, 2025
8cb9a2c
#409: address minor review comments
nmm0 Jun 18, 2025
24f1bba
#409: adjust condition for checking representability of range
nmm0 Jun 19, 2025
738f357
#409: test precondition violation message on host
nmm0 Jun 25, 2025
8a5a707
Merge branch 'stable' into 409-check-preconditions-in-extents-constru…
crtrott Feb 13, 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
31 changes: 26 additions & 5 deletions 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
61 changes: 61 additions & 0 deletions include/experimental/__p0009_bits/utility.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,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 +214,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;
}
}

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
2 changes: 1 addition & 1 deletion tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ mdspan_add_test(test_macros ENABLE_PRECONDITIONS)
mdspan_add_test(test_layout_preconditions ENABLE_PRECONDITIONS)

mdspan_add_test(test_dims)
mdspan_add_test(test_extents)
mdspan_add_test(test_extents ENABLE_PRECONDITIONS)
mdspan_add_test(test_mdspan_at)
mdspan_add_test(test_mdspan_ctors)
mdspan_add_test(test_mdspan_swap)
Expand Down
78 changes: 78 additions & 0 deletions tests/test_extents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,3 +481,81 @@ TEST(TestExtentsCTADStdArray, test_extents_ctad_std_array) {
}
*/
#endif

#if MDSPAN_HAS_CXX_17
TEST(TestExtentsConstructorPreconditions, test_extents_construct_indices) {
auto test_precondition_indices_not_representable = [] {
[[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ 500, 500 };
};
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_indices_not_representable(), "");
Copy link
Member

@dalg24 dalg24 Jun 16, 2025

Choose a reason for hiding this comment

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

We do not want to ensure that the error message matches some regex?

Copy link
Contributor Author

@nmm0 nmm0 Jun 18, 2025

Choose a reason for hiding this comment

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

We can at least for CPU but it won't work with CUDA. So I just decided not to since it doesn't matter a whole lot

Copy link
Member

@dalg24 dalg24 Jun 18, 2025

Choose a reason for hiding this comment

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

What do you mean it won't work with CUDA?
We do check error messages from the device in Kokkos so there is some level of support.

Copy link
Contributor Author

@nmm0 nmm0 Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

So for now the CUDA precondition violation handler prints to stdout. EXPECT_DEATH checks stderr -- we could do something like:

#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
  EXPECT_DEATH(test_precondition_indices_not_representable(), "");
#else
  EXPECT_DEATH(test_precondition_indices_not_representable(), "extent_is_representable");
#endif

We do this elsewhere. But really the precondition handler doesn't print the best errors anyway

Copy link
Contributor Author

@nmm0 nmm0 Jun 25, 2025

Choose a reason for hiding this comment

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

I updated to test the message on host. On device the message is kinda useless:

void Kokkos::detail::default_precondition_violation_handler(const char*, const char*, unsigned int): Assertion `0' failed.

So I think we should avoid testing device messages for now until we update our precondition violation handler on device (likely a separate PR)

Copy link
Member

@dalg24 dalg24 Jun 30, 2025

Choose a reason for hiding this comment

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

Oh, I see, because we call printf(...) and assert(false) instead of __assert_fail(...) like we do in Kokkos

#else
EXPECT_DEATH(test_precondition_indices_not_representable(), "all_values_are_nonnegative_and_representable");
#endif

auto test_precondition_indices_are_negative = [] {
[[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ 500, -500 };
};
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_indices_are_negative(), "");
#else
EXPECT_DEATH(test_precondition_indices_are_negative(), "all_values_are_nonnegative_and_representable");
#endif
}

TEST(TestExtentsConstructorPreconditions, test_extents_construct_array) {
auto test_precondition_array_elements_not_representable = [] {
[[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ std::array{ 500, 500 } };
};
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_array_elements_not_representable(), "");
#else
EXPECT_DEATH(test_precondition_array_elements_not_representable(), "range_is_nonnegative_and_representable");
#endif

auto test_precondition_array_elements_are_negative = [] {
[[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ std::array{ 500, -500 } };
};
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_array_elements_are_negative(), "");
#else
EXPECT_DEATH(test_precondition_array_elements_are_negative(), "range_is_nonnegative_and_representable");
#endif
}

#ifdef __cpp_lib_span
TEST(TestExtentsConstructorPreconditions, test_extents_construct_span) {
auto test_precondition_span_elements_not_representable = [] {
auto indices = std::array{ 500, 500 };
[[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ std::span{ indices } };
};
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_span_elements_not_representable(), "");
#else
EXPECT_DEATH(test_precondition_span_elements_not_representable(), "range_is_nonnegative_and_representable");
#endif

auto test_precondition_span_elements_are_negative = [] {
auto indices = std::array{ 500, -500 };
[[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ std::span{ indices } };
};
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_span_elements_are_negative(), "");
#else
EXPECT_DEATH(test_precondition_span_elements_are_negative(), "range_is_nonnegative_and_representable");
#endif
}
#endif

TEST(TestExtentsConstructorPreconditions, test_extents_construct_other_extents) {
auto test_precondition_extent_ranks_not_representable = [] {
auto first = Kokkos::dextents< int, 2 >{ 500, 500 };
[[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ first };
};
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_extent_ranks_not_representable(), "");
#else
EXPECT_DEATH(test_precondition_extent_ranks_not_representable(), "extent_is_representable");
#endif
}
#endif