Skip to content

Conversation

vitor1001
Copy link

See https://github.com/rapidsai/raft/blob/branch-25.08/cpp/include/raft/thirdparty/mdspan/include/experimental/__p0009_bits/aligned_accessor.hpp.

This implementation was adapted from kokkos/mdspan/blob/stable/examples/aligned_accessor/aligned_accessor.cpp.

Fixes #412.

Copy link
Contributor

@nmm0 nmm0 left a comment

Choose a reason for hiding this comment

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

Thank you for submitting this. This is still also missing some tests and is_sufficiently_aligned

This was on my TODO list -- if you would like, I can take this over or I can continue to review this.

@@ -0,0 +1,783 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove this file as we already have padded layouts

Comment on lines +103 to +132
constexpr bool
is_nonzero_power_of_two(const std::size_t x)
{
// Just checking __cpp_lib_int_pow2 isn't enough for some GCC versions.
// The <bit> header exists, but std::has_single_bit does not.
#if defined(__cpp_lib_int_pow2) && __cplusplus >= 202002L
return std::has_single_bit(x);
#else
return x != 0 && (x & (x - 1)) == 0;
#endif
}

template<class ElementType>
constexpr bool
valid_byte_alignment(const std::size_t byte_alignment)
{
return is_nonzero_power_of_two(byte_alignment) && byte_alignment >= alignof(ElementType);
}

// We define aligned_pointer_t through a struct
// so we can check whether the byte alignment is valid.
// This makes it impossible to use the alias
// with an invalid byte alignment.
template<class ElementType, std::size_t byte_alignment>
struct aligned_pointer {
static_assert(valid_byte_alignment<ElementType>(byte_alignment),
"byte_alignment must be a power of two no less than "
"the minimum required alignment of ElementType.");
using type = ElementType* _MDSPAN_ALIGN_VALUE_ATTRIBUTE( byte_alignment );
};
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these guys should be in a detail namespace

@@ -0,0 +1,186 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Just reminding ourselves we need to fix this license as it is not the correct one anymore

Comment on lines +138 to +143
template<class ElementType, std::size_t byte_alignment>
aligned_pointer_t<ElementType, byte_alignment>
bless(ElementType* ptr, std::integral_constant<std::size_t, byte_alignment> /* ba */ )
{
return _MDSPAN_ASSUME_ALIGNED( ElementType, ptr, byte_alignment );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
template<class ElementType, std::size_t byte_alignment>
aligned_pointer_t<ElementType, byte_alignment>
bless(ElementType* ptr, std::integral_constant<std::size_t, byte_alignment> /* ba */ )
{
return _MDSPAN_ASSUME_ALIGNED( ElementType, ptr, byte_alignment );
}

@vitor1001
Copy link
Author

Thank you for submitting this. This is still also missing some tests and is_sufficiently_aligned

This was on my TODO list -- if you would like, I can take this over or I can continue to review this.

From my side I prefer you take this over. You know obviously much better this codebase than I do.

Apply suggestions

Co-authored-by: Nicolas Morales <nmmoral@sandia.gov>
@nmm0
Copy link
Contributor

nmm0 commented Jun 20, 2025

Thank you for submitting this. This is still also missing some tests and is_sufficiently_aligned
This was on my TODO list -- if you would like, I can take this over or I can continue to review this.

From my side I prefer you take this over. You know obviously much better this codebase than I do.

Ok I will do, I really appreciate your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: support aligned_accessor for C++20 code
2 participants