MDSpan issues expose by Kokkos View refactor #358
Conversation
| @@ -95,6 +95,7 @@ struct padded_extent { | |||
| using static_array_type = typename static_array_type_for_padded_extent< | |||
| padding_value, _Extents, _ExtentToPadIdx, _Extents::rank()>::type; | |||
|
|
|||
| MDSPAN_INLINE_FUNCTION | |||
There was a problem hiding this comment.
Do you actually need this or did you just add for consistency?
There was a problem hiding this comment.
static_value is called inside a couple constructors and is_always_exhaustive all of which are device callable. The "INLINE" is just for consistency, at some point we can go wholesale through this stuff and replace it all with MDSPAN_FUNCTION
| @@ -136,7 +137,7 @@ struct padded_extent { | |||
| return {}; | |||
| } | |||
| // Missing return statement warning from NVCC and ICC | |||
| #if defined(__NVCC__) || defined(__INTEL_COMPILER) | |||
| #if (defined(__NVCC__) || defined(__INTEL_COMPILER)) && !defined(__NVCOMPILER) | |||
| return {}; | |||
| #endif | |||
There was a problem hiding this comment.
All of these are contained in #356 and should disappear after rebasing.
| @@ -203,7 +204,7 @@ class layout_left_padded<PaddingValue>::mapping { | |||
| } | |||
|
|
|||
| public: | |||
| #if !MDSPAN_HAS_CXX_20 | |||
| #if !MDSPAN_HAS_CXX_20 || defined(__NVCC__) | |||
There was a problem hiding this comment.
What's the problem here?
There was a problem hiding this comment.
NVCC does not like the requires stuff down there. Basically, it ends up saying that there is not default constructor on the device available in C++20 mode.
There was a problem hiding this comment.
I wasn't able to narrow it down in a simple reproducer.
| @@ -52,6 +53,33 @@ template <class OffsetType, class ExtentType, class StrideType> | |||
| struct is_strided_slice< | |||
| strided_slice<OffsetType, ExtentType, StrideType>> : std::true_type {}; | |||
|
|
|||
| // Helper for identifying valid pair like things | |||
| template <class T, class IndexType> struct index_pair_like : std::false_type {}; | |||
There was a problem hiding this comment.
The feature macro for complex get support is called __cpp_lib_tuple_like so maybe
| template <class T, class IndexType> struct index_pair_like : std::false_type {}; | |
| template <class T, class IndexType> struct index_tuple_like : std::false_type {}; |
There was a problem hiding this comment.
this has nothing to do with get: https://eel.is/c++draft/views.multidim#mdspan.syn
We are using an exposition only concept called index-pair-like and if you follow all the stuff down to their roots, this thing here basically implements that concept in form of a type_trait.
There was a problem hiding this comment.
The reason why I looked at get was that that's what is used in first_of which now uses this concept as requirement.
There was a problem hiding this comment.
I fixed that by adding special versions for complex.
…ble extents Specifically the 1D layout_left_padded <-> layout_right_padded ctors didn't compile for cases where the new mapping has a static extent, but the source mapping has dynamic extent.
crtrott
force-pushed
the
some-mdspan-fixes
branch
from
7da7385
to
192ffb2
Compare
and test that complex<double> works like pair as slice specifier *barf*
| static constexpr bool value = std::is_convertible_v<IdxT, IndexType>; | ||
| }; | ||
|
|
||
| // FIXME: we actually need to pass IndexType into all of these |
|
As discussed elsewhere: complex is one of the named types for which this is supposed to work based on the definition of index-pair-like I see no reason to not include it here in the back port. Every standard approved version of submdspan will take complex as slice argument, and so I don't see a good reason to exclude it from this back port to C++17. |
layout_left_paddded <-> layout_right_paddedconversion didn't work if theextents_typeare not convertibleRelated to: kokkos/kokkos#7288