Skip to content

Avoid calling memcpy in swap #5500

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

Merged
merged 8 commits into from
May 17, 2025

Conversation

AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented May 12, 2025

Fixes #5481

The release codegen is the same, the benchmark results are the same.

memcpy calls are now not emitted in any mode. In /Od we have rep movsb, which is pretty good for a debug mode.

unsigned char is kept for underlying type as it doesn't have alignment requirements, and also is a valid alias for any object.

@frederick-vs-ja pointed out that the strict aliasing rule is still violated here. I excluded Clang (and thus Intel), I verified Clang already optimizes the usual swap loop on its own. MSVC doesn't take advantage of strict aliasing, so it is fine 🔥🐶☕🔥

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner May 12, 2025 07:46
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews May 12, 2025
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label May 12, 2025
@StephanTLavavej StephanTLavavej self-assigned this May 12, 2025
@StephanTLavavej StephanTLavavej removed their assignment May 15, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in STL Code Reviews May 15, 2025
@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews May 16, 2025
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 2eb069f into microsoft:main May 17, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Merging to Done in STL Code Reviews May 17, 2025
@StephanTLavavej
Copy link
Member

🔁 💱 🐱

@AlexGuteniev AlexGuteniev deleted the dont-say-memcpy-mean-it branch May 18, 2025 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

<concepts>: std::swap for arrays uses memcpy
3 participants