ORNL / cpp-proposals-pub

Collaborating on papers for the ISO C++ committee - public repo
26 stars 26 forks source link

P2897R1 (aligned_accessor): LEWG presentation 2024/06 #458

Closed mhoemmen closed 1 month ago

mhoemmen commented 2 months ago

P2897R1 (aligned_accessor)

Brief example with a complete implementation: https://godbolt.org/z/YsnsYhvGc

Last LEWG review

LEWG reviewed P2897R0 on 2023/10/10. Feedback:

  1. Switch to Mandates elements for alignment conversions
  2. Discuss lack of explicit ctor from default_accessor
  3. Authors will explore mdspan safety, in the post-kona timeframe

Switch to Mandates elements for alignment conversions

DONE. See [mdspan.accessor.aligned.members] para 2.

Discuss lack of explicit ctor from default_accessor

DONE. We added an explicit constructor aligned_accessor(default_accessor<OtherElementType>). (Accessor constructors with preconditions are explicit.)

Authors will explore mdspan safety, in the post-kona timeframe

DONE (at least we think so).

Added a function to check the data handle's precondition

Added constexpr static bool is_sufficiently_aligned(data_handle_type p) member function. This lets users check the data handle's precondition before constructing the mdspan. Before this addition, users had to implement their own precondition check.

aligned_accessor does not introduce more sharp edges

The proposed aligned_accessor introduces no more sharp edges than the C++20 feature assume_aligned, which was not part of the mdspan series of proposals. Similarly, default_accessor introduces no more sharp edges than C++ already has with raw pointer access. Checking the precondition of default_accessor::access means checking whether a pointer plus an offset is valid. C++ currently offers no way to do that in general.

Regarding the precondition of default_accessor::access, we considered adding a new Accessor type whose data_handle_type is span<ElementType, Extent> instead of ElementType*. However, that would just exchange the pointer precondition for another, namely that mapping.required_span_size() <= data_handle.size(). Users can also implement this Accessor type themselves if they wish.

mdspan's constructor cannot generically check the data handle's preconditions

Accessors' constructors do not have access to the mdspan's data handle. The first time an Accessor gets the data handle is in the Accessor's access function, after both the Accessor and the mdspan have been constructed. Also, mdspan generally permits all kinds of Accessors. Their data handles does not need to represent a contiguous range or refer to objects in memory. Accessors may wrap third-party libraries that have no way to check validity of handles until use. As a consequence, mdspan's constructor cannot generically check the data handle's preconditions. Implementations could always add checks to their specializations of mdspan for Standard accessors like default_accessor and the proposed aligned_accessor.

Why it belongs in the Standard

Analogous to atomic_accessor_*

History

Key features

Questions

Completeness checklist

Implementation experience

Implemented as an example in the reference mdspan implementation.

Demo: https://godbolt.org/z/fGb68oY67 (code generation for raw pointers + std::assume_aligned is the same as for mdspan with aligned_accessor)

mhoemmen commented 2 months ago

LEWG review 2024/06/28

Forwarded to LWG.

Required change

Remove constexpr from is_sufficiently_aligned, because bit_cast<uintptr_t>(ptr) is never a constant expression.

Suggestions

Explain unified approach to possibly unsafe mdspan conversions

Explain that mdspan and all its components have a unified approach to unsafe conversions, which is explicit constructors. Thus, making unsafe conversions only happen through some cast operator (unsafe_mdspan_cast) would not make existing code safer.

// Assuming aligned_mdspan alias from proposal's example,
// and the following function:
extern void f(aligned_mdspan<8>);

// with naughty_cast

auto aligned = aligned_mdspan<8>(
  unaligned.data_handle(),
  unaligned.mapping(),
  naughty_cast<aligned_accessor<float, 8>>(unaligned.accessor()));
f(aligned);

// or briefly
f(naughty_cast<aligned_mdspan<8>>(unaligned));

// without naughty_cast

f(aligned_mdspan<8>(unaligned));

Checking preconditions of accessor in generic code?

There was a question whether is_sufficiently_aligned could be generic -- if it would make sense for accessors and perhaps also layout mappings to have a static bool member function for checking preconditions. That would let generic code check preconditions for generic accessors. This shouldn't be called is_valid_* because we can't e.g., ask at run time if a pointer is valid. On the other hand, it could make sense to make a best-effort check. My only objection is that this is the first of several accessors that even has some precondition that could be checked.

Add explanation why is_sufficiently_aligned can't be constexpr

Link to discussion example

https://godbolt.org/z/cWMaT3d8n

mhoemmen commented 1 month ago

PR https://github.com/ORNL/cpp-proposals-pub/pull/461 has P2897R2, which I submitted on 2024/07/12.