Skip to content
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.

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

Change MDSPAN_INLINE_FUNCTION to _MDSPAN_INLINE_FUNCTION (was "Rename _MDSPAN_HOST_DEVICE to MDSPAN_HOST_DEVICE") #185

Open
mhoemmen opened this issue Sep 2, 2022 · 0 comments

Comments

@mhoemmen
Copy link
Contributor

mhoemmen commented Sep 2, 2022

@youyu3 @crtrott

New issue

I revised this issue on 2022/09/06, based on offline discussion with @crtrott . We do not want to give users the impression that they are allowed to use the MDSPAN_INLINE_FUNCTION macro. It is for mdspan implementers only. It may be used in mdspan tests and examples, but it may NOT be used outside mdspan.

Thus, we need to add an underscore prefix to the MDSPAN_INLINE_FUNCTION macro, to make it clear to users that it is an implementation detail and not for use outside mdspan. This will make it consistent with the _MDSPAN_HOST_DEVICE macro, which must retain its underscore prefix for the same reason.

The former text of this issue is below, and no longer applies.

Former issue

Based on my comment here: #184 (comment)

Rename _MDSPAN_HOST_DEVICE to MDSPAN_HOST_DEVICE, or add a new MDSPAN_HOST_DEVICE macro that has the same definition as _MDSPAN_HOST_DEVICE. This would promote the macro to be just as usable as MDSPAN_INLINE_FUNCTION. (An underscore prefix suggests that it is an implementation detail.) Here are some reasons.

  1. It's not correct to apply the inline keyword to lambdas.
  2. In fact, existing tests already use _MDSPAN_HOST_DEVICE for the lambdas given to dispatch. This suggests to users that _MDSPAN_HOST_DEVICE is not really an implementation detail.
  3. Some mdspan implementation strategies (e.g., []<size_t ...>{ /* stuff */ }) depend on lambdas, so we need a "blessed" way to declare lambdas __host__ __device__.
  4. The inline keyword changes ODR semantics. Forcing use of inline with __host__ __device__, while not so important for a header-only library, could be bad for users' code that relies on RDC or other vendors' equivalent constructs.
@mhoemmen mhoemmen changed the title Rename _MDSPAN_HOST_DEVICE to MDSPAN_HOST_DEVICE Change MDSPAN_INLINE_FUNCTION to _MDSPAN_INLINE_FUNCTION (was "Rename _MDSPAN_HOST_DEVICE to MDSPAN_HOST_DEVICE") Sep 6, 2022
mhoemmen added a commit to mhoemmen/mdspan that referenced this issue Jul 26, 2023
…s_impl

[ready] kokkos impl `vector_norm2`
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

No branches or pull requests

1 participant