Skip to content

Further cleanup of vectorized minmax_element - #6459

Merged
Stephan T. Lavavej (StephanTLavavej) merged 9 commits into
microsoft:mainfrom
AlexGuteniev:minmax-clarity
Oct 1, 2026
Merged

Stephan T. Lavavej (StephanTLavavej) merged 9 commits into
microsoft:mainfrom
AlexGuteniev:minmax-clarity

Conversation

@AlexGuteniev

Copy link
Copy Markdown
Contributor

Follow up to #6450

Reduce oddness of minmax_element:

  • Avoid using preprocessor to differentiate between ARM64 & ARM64EC vs x64 & x86. We actually care about _Has_unsigned_cmp. Which may become true for x64 & x86 too, if we make use of AVX-512
  • Use numeric_limits instead of having initialization values from traits
  • Use is_floating_point_v instead of propagating this information via traits

Copilot AI balanced review requested due to automatic review settings September 21, 2026 17:17
@github-project-automation github-project-automation Bot moved this to Initial Review in STL Code Reviews Sep 21, 2026
@AlexGuteniev

Alex Guteniev (AlexGuteniev) commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

I'd like to have one more cleanup here to get rid of this sizeof(_Ty) == 8 special case:

// We don't have unsigned 64-bit stuff, so we'll use sign correction just for that case
constexpr bool _Sign_correction = sizeof(_Ty) == 8 && !_Is_signed && !_Traits::_Has_unsigned_cmp;

I think that would go to a separate PR.

@azure-pipelines

This comment was marked as resolved.

This comment was marked as resolved.

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

This comment was marked as resolved.

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) moved this from Initial Review to Ready To Merge in STL Code Reviews Sep 23, 2026
It will go without sign correction.
Very minor perf change of an unimportant case,
but I want the traits to be honest
Copilot AI review requested due to automatic review settings September 25, 2026 19:53

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) merged commit b0459ef into microsoft:main Oct 1, 2026
48 checks passed
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews Oct 1, 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