kokkos / kokkos/mdspan

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

Open
#185 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
507
Forks
87
Avg merge
15h 27m
Merged PRs (30d)
2

Description

@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: https://github.com/kokkos/mdspan/pull/184#issuecomment-1235715279

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.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Search the repository for MDSPAN_INLINE_FUNCTION and review each definition, use, test, and example affected by the rename. Confirm that the implementation-only macro consistently uses the underscore-prefixed name, that the old name has no remaining references, and run the repository's relevant C++ tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.