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

simd: add simd_size_type #6535

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

ldh4
Copy link
Contributor

@ldh4 ldh4 commented Oct 24, 2023

Related to #5674 and #6109 (comment).

Introduced an alias, simd_size_type, for a signed int type.
Replaced size_t and int uses by simd_size_type as described in p1928r7 (section 4.10):

Different to the TS, this paper uses simd-size-type instead of size_t for
• the SIMD width (number of elements),
• the generator constructor call argument,
• the subscript operator arguments, and
...

@nliber
Copy link
Contributor

nliber commented Nov 1, 2023

#6109 (comment) was asking for a simd_size_type to be a private type. This change makes it a public type. Is that intentional?

I guess I'm asking whether or not this should be in Kokkos::Impl (or equivalent) or not.

I don't know that the answer is obvious one way or the other. For instance, simd_mask<T>::size() right now returns it, which would argue in favor of it being public. (If it were private, then simd_mask<T> ought to have a typedef for it.)

@ldh4
Copy link
Contributor Author

ldh4 commented Nov 2, 2023

#6109 (comment) was asking for a simd_size_type to be a private type. This change makes it a public type. Is that intentional?

I guess I'm asking whether or not this should be in Kokkos::Impl (or equivalent) or not.

I don't know that the answer is obvious one way or the other. For instance, simd_mask<T>::size() right now returns it, which would argue in favor of it being public. (If it were private, then simd_mask<T> ought to have a typedef for it.)

It was also unclear to me whether placing simd_size_type in an impl namespace provides a value. As you described, there will need an alias for Impl::simd_size_type to be used publicly in all backends. (Or using simd_size_type = Impl::internal_simd_size_type, if we want to preserve the names as consistently as possible with the synopsis provided in p1928r7 section 7.1.2)

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.

None yet

2 participants