Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Align complex types to total size of both components #161

Closed
wants to merge 1 commit into from

Conversation

wmaxey
Copy link
Member

@wmaxey wmaxey commented Apr 27, 2021

No description provided.

@wmaxey wmaxey requested review from ogiroux and griwes April 27, 2021 20:03
Copy link
Collaborator

@griwes griwes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alignment must be conditional on the abi version. See

# if _LIBCUDACXX_CUDA_ABI_VERSION < 3
for prior art.

@griwes
Copy link
Collaborator

griwes commented Apr 27, 2021

Wouldn't a simpler change of just slapping alignas(2*sizeof(T)) on complex itself work?

@griwes
Copy link
Collaborator

griwes commented Apr 27, 2021

This also needs tests that verify the alignments for both the old abi versions and the new one; there's a barrier v2 abi test in heterogeneous tests that you can use as prior art for how to approach it in general.

@wmaxey
Copy link
Member Author

wmaxey commented Apr 27, 2021

Wouldn't a simpler change of just slapping alignas(2*sizeof(T)) on complex itself work?

Funny, I hadn't thought of that. Originally I had wrapped the members in anonymous structs, but NVRTC doesn't support that extension.

@wmaxey wmaxey force-pushed the bugfix/complex_alignment branch from da7a418 to c7446b3 Compare April 27, 2021 20:51
@wmaxey wmaxey force-pushed the bugfix/complex_alignment branch from c7446b3 to 3896f6b Compare April 27, 2021 21:06
@wmaxey wmaxey requested a review from griwes April 27, 2021 21:06
@wmaxey
Copy link
Member Author

wmaxey commented Apr 27, 2021

  • Simplified implementation
  • Added ABI version tests
  • Currently passes on GCC, Clang, and NVRTC builds
    • MSVC TBD

@wmaxey wmaxey added this to the 1.6.0 milestone May 18, 2021
@brycelelbach brycelelbach added P0: must have Absolutely must ship with the milestone. bug: performance Does not perform as intended. labels May 18, 2021
@wmaxey
Copy link
Member Author

wmaxey commented Jul 8, 2021

Closing as #172 will supersede this PR.

@wmaxey wmaxey closed this Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug: performance Does not perform as intended. P0: must have Absolutely must ship with the milestone.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants