Skip to content

Vectorized minmax_element: reduce insanity of sign correction - #6450

Merged
Stephan T. Lavavej (StephanTLavavej) merged 2 commits into
microsoft:mainfrom
AlexGuteniev:avoid-runtime-sign-dispatch
Sep 16, 2026
Merged

Stephan T. Lavavej (StephanTLavavej) merged 2 commits into
microsoft:mainfrom
AlexGuteniev:avoid-runtime-sign-dispatch

Conversation

@AlexGuteniev

Copy link
Copy Markdown
Contributor

Avoids passing sign as runtime parameter for SSE4.2 and AVX2 vectorized minmax_element.

Originally it has been done to reduce code bloat without any performance loss. SSE4.2 and AVX2 lack some unsigned instructions, so unsigned operations has to be implemented via signed ones. We applied sign correction at runtime, and shared import lib functions for signed and unsigned types.

Now that Neon has unsigned instructions, we have different entry points for signed and unsigned types to avoid runtime branches there We use compile-time sign dispatch there.. AVX-512, if implemented, will also take advantage of having unsigned instructions and will use comile-time dispatch.

This PR unifies minmax_element sign dispatch to make it compile-time everywhere.
It also renames _Sign to _Is_signed to reduce confusing.

Achivements:

  • Simplification
  • Unification with _Sorted_ranges
  • No performance gain or loss. Some gain could have been expected, especially for signed types, but if there is such a gain, it does not stand away enough from the noise
  • Some code bloat. Not measured

Copilot AI balanced review requested due to automatic review settings September 14, 2026 06:16
@github-project-automation github-project-automation Bot moved this to Initial Review in STL Code Reviews Sep 14, 2026
@azure-pipelines

This comment was marked as resolved.

This comment was marked as resolved.

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added the enhancement Something can be improved label Sep 14, 2026
Comment thread stl/src/vector_algorithms.cpp Outdated
Copilot AI review requested due to automatic review settings September 14, 2026 17:50
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks, I really like this simplification! 😻

This comment was marked as resolved.

@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 Stephan T. Lavavej (StephanTLavavej) moved this from Ready To Merge to Merging in STL Code Reviews Sep 15, 2026
@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 305d43f into microsoft:main Sep 16, 2026
48 checks passed
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews Sep 16, 2026
@StephanTLavavej

Copy link
Copy Markdown
Member

😻 🪄 💖

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