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

Fix alignment for Kokkos::complex #2255

Closed
crtrott opened this issue Aug 28, 2019 · 10 comments
Closed

Fix alignment for Kokkos::complex #2255

crtrott opened this issue Aug 28, 2019 · 10 comments
Assignees
Labels
Enhancement Improve existing capability; will potentially require voting

Comments

@crtrott
Copy link
Member

crtrott commented Aug 28, 2019

Reported at hackathon, in order to get proper 8 byte and 16 byte loads for complex<float> and complex<double> the user derived his own classes from CUDA's float2 and double2.

But it is enough to tell the type that it is aligned: alignas ( 2*sizeof(RealType))

@crtrott crtrott added the Enhancement Improve existing capability; will potentially require voting label Aug 28, 2019
@crtrott
Copy link
Member Author

crtrott commented Aug 28, 2019

Needs to fix both pair and complex

@dhollman
Copy link

Don't you mean 2*alignof(T)? Especially for pair, using sizeof could be a huge cost.

@nliber
Copy link
Contributor

nliber commented Aug 28, 2019

And that won't work for pair, as it takes two different types

@dhollman
Copy link

I actually think we don't even want to go there with pair. When T and U are different, alignof do you use? If T is first, you will always get alignof(pair<T, U>) >= alignof(T). But if alignof(U) > alignof(T), what should the default behavior be?

@dhollman
Copy link

@nliber jinx

@crtrott
Copy link
Member Author

crtrott commented Aug 29, 2019

We can partially specialize for pair<T,T>.

@dhollman
Copy link

dhollman commented Aug 29, 2019

I don't know if we really want to support use cases that abuse pair in this way, but if we do, we should go all in on it:

template <class T, class U>
struct alignas((alignof(T) > alignof(U)) ? 2 * alignof(T) : 2 * alignof(U)) pair;

That should do the trick, and should support whatever use cases we would have wanted to support for the pair<T, T> case

@nliber
Copy link
Contributor

nliber commented Aug 29, 2019

There look to be a fair number of uses of pair<T, T> inside Kokkos, so we probably should do this (the full general one @dhollman wrote above).

@crtrott
Copy link
Member Author

crtrott commented Aug 29, 2019

I am fine with that.

@ndellingwood
Copy link
Contributor

Related PR #2259

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

5 participants