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

Initially use [[msvc::no_unique_address]] for some C++23 components #4960

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Sep 17, 2024

Towards #1364.

Applied:

  • in_value_result
  • out_value_result
  • mdspan
  • join_with_view's sentinel
  • enumerate_view's iterator and sentinel
  • chunk_view's iterator
  • slide_view's sentinel

Not yet applied due to DevCom-10588147 and possibly related issues.

  • views, as they have view_interface as their empty bases.
  • some iterators with empty bases due to conditionally existing iterator_category.
  • chunk_by_view's iterator, possibly because the non-potentially-overlapping member is listed first.

Not yet applied because it's unclear to me how to reuse padding bytes in tuple

  • zip-family

Not yet applied due to another unclear reasons

  • expected

Other changes:

  • remove /* [[no_unique_address]] */ for _Index_type of repeat_view's iterator because the _Index_type never has padding bytes
  • move the test coverage added in <mdspan>: More EBO #3847 to a separated test file
  • attempt to test NVCC for these changes failed and reverted.

- `in_value_result`
- `out_value_result`
- `mdspan`
- `join_with_view`'s sentinel
- `enumerate_view`'s iterator and sentinel
- `chunk_view`'s iterator
- `slide_view`'s sentinel
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner September 17, 2024 16:05
@frederick-vs-ja

This comment was marked as resolved.

@CaseyCarter CaseyCarter added performance Must go faster decision needed We need to choose something before working on this labels Sep 17, 2024
@CaseyCarter
Copy link
Member

I'm going to mark this "decision needed" so we can discuss it in the maintainer meeting this week. We need to decide how we're going to communicate to Nvidia and Intel that this is going to happen and that they'll need to implement this part of the ABI to travel into the future with us.

@StephanTLavavej StephanTLavavej removed the decision needed We need to choose something before working on this label Sep 18, 2024
@StephanTLavavej
Copy link
Member

I've communicated this upcoming change to our contacts at NVIDIA. As CUDA 12.4.0 doesn't support C++23, you can revert your experimental test change here.

We don't officially support Intel's compiler, but as it's Clang-based we expect that it won't have problems understanding this attribute.

@StephanTLavavej StephanTLavavej self-assigned this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
Status: Initial Review
Development

Successfully merging this pull request may close these issues.

3 participants