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 14 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
  • elements_of

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

This comment was marked as resolved.

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

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej self-assigned this Sep 19, 2024
@StephanTLavavej

This comment was marked as resolved.

@CaseyCarter

This comment was marked as resolved.

@frederick-vs-ja

This comment was marked as resolved.

@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej removed their assignment Oct 20, 2024
@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej added the blocked Something is preventing work on this label Oct 21, 2024
@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej marked this pull request as draft October 26, 2024 14:56
@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej

This comment was marked as outdated.

@StephanTLavavej

This comment was marked as outdated.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 31, 2025

VSO-2355645 DevCom-10837331 "std::set broken under VS 17.12.4 with no_unique_address comparator" may affect this PR. A fix was merged for 17.14 Preview 3.

@StephanTLavavej

This comment was marked as resolved.

@frederick-vs-ja frederick-vs-ja marked this pull request as ready for review February 20, 2025 11:03
@StephanTLavavej
Copy link
Member

Internal MSVC-PR-608181 has finally updated the toolset to 17.14 Preview 1 in MSVC main, picking up the fix for DevCom-10754777 "Wrong code generation when initializing a variable of class type that contains multiple instances of the same empty class in its layout".

Now we're just waiting for MSVC main to be merged into MSVC prod/fe which happens fairly frequently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Something is preventing work on this performance Must go faster
Projects
Status: Blocked
Development

Successfully merging this pull request may close these issues.

3 participants