Skip to content

vector_algorithms.cpp: Add [[nodiscard]] to _Use_avx2() and _Use_sse42()#6192

Merged
StephanTLavavej merged 2 commits intomicrosoft:mainfrom
jovibor:main
Mar 31, 2026
Merged

vector_algorithms.cpp: Add [[nodiscard]] to _Use_avx2() and _Use_sse42()#6192
StephanTLavavej merged 2 commits intomicrosoft:mainfrom
jovibor:main

Conversation

@jovibor
Copy link
Copy Markdown
Contributor

@jovibor jovibor commented Mar 30, 2026

Adds [[nodiscard]] to _Use_avx2() and _Use_sse42() functions.

@jovibor jovibor requested a review from a team as a code owner March 30, 2026 09:55
@github-project-automation github-project-automation Bot moved this to Initial Review in STL Code Reviews Mar 30, 2026
Comment thread stl/src/vector_algorithms.cpp Outdated
@github-project-automation github-project-automation Bot moved this from Initial Review to Work In Progress in STL Code Reviews Mar 30, 2026
@YexuanXiao
Copy link
Copy Markdown
Contributor

YexuanXiao commented Mar 30, 2026

The query results can be cached as std::atomic<bool>s with namespace scope and internal linkage. This pattern helps generate cleaner and more stable code than magic static.

@YexuanXiao
Copy link
Copy Markdown
Contributor

YexuanXiao commented Mar 30, 2026

MSVC can optimize the function-scoped static atomic to be the same as one with internal linkage at namespace scope, without using the magic static mechanism, and magic static is worse, see: https://godbolt.org/z/r4zr9oE7s

@StephanTLavavej
Copy link
Copy Markdown
Member

No - <atomic> is a non-core header, which we can't drag into the import lib. See #3011.

Aside from that, the complexity of introducing atomics isn't worth it here. We're reading an integer, not doing anything outrageously expensive, and branch prediction is going to be really happy with a result that doesn't change.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Mar 30, 2026
@jovibor
Copy link
Copy Markdown
Contributor Author

jovibor commented Mar 30, 2026

Dragging-in atomics for such a tiny function would definitely be an overkill, imo.
I reverted changes back, leaving only [[nodiscard]].

@StephanTLavavej
This introduces magic statics, which we avoid (they have unusual interactions with certain code).

Can you give any hints, what exactly these unusual interactions is, or where to read about it?
As far as I know the "magic statics" are guaranteed to be thread-safe and one-time initialized since C++11.

@StephanTLavavej StephanTLavavej changed the title <vector_algorithms.cpp> Add static and [[nodiscard]] to _Use_avx2() and _Use_sse42() vector_algorithms.cpp: Add [[nodiscard]] to _Use_avx2() and _Use_sse42() Mar 31, 2026
@StephanTLavavej StephanTLavavej moved this from Work In Progress to Ready To Merge in STL Code Reviews Mar 31, 2026
@StephanTLavavej
Copy link
Copy Markdown
Member

As I recall, they create a TLS section in the resulting binary, which had weird interactions with an optimization that Windows wanted to do.

@YexuanXiao
Copy link
Copy Markdown
Contributor

YexuanXiao commented Mar 31, 2026

I have re-showd my comments, which compare magic static and atomic. The conclusion is that if you want the best performance, then use lock-free atomics, or do not cache the result. Although I have only shown the results for MSVC, this actually applies to all compilers and operating systems.

@StephanTLavavej
Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo. Please notify me if any further changes are pushed, otherwise no action is required.

@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Mar 31, 2026
@StephanTLavavej StephanTLavavej merged commit 054d33a into microsoft:main Mar 31, 2026
49 checks passed
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews Mar 31, 2026
@StephanTLavavej
Copy link
Copy Markdown
Member

Thanks for adding our favorite attribute! 😻

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.

3 participants