Skip to content

Fix ARM64/ARM64EC minmax_element() family vectorization for unsigned elements #6376

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.

Sign up for GitHub

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

Open
StephanTLavavej wants to merge 2 commits into
base: main
Choose a base branch
from
+52 −3
Open

Fix ARM64/ARM64EC minmax_element() family vectorization for unsigned elements #6376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
153a69e
Add test coverage.
StephanTLavavej Jul 22, 2026
dfab9fb
Fix bug. (By Opus 4.8, cleaned up by STL.)
StephanTLavavej Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions stl/src/vector_algorithms.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2970,6 +2970,13 @@ namespace {
auto _Cur_min_val = _Traits::_Init_min_val;
auto _Cur_max_val = _Traits::_Init_max_val;
Comment thread
StephanTLavavej marked this conversation as resolved.

#if defined(_M_ARM64) || defined(_M_ARM64EC)
if constexpr (!_Sign && _Traits::_Has_unsigned_cmp) {
_Cur_min_val = -1;
_Cur_max_val = 0;
}
#endif // ^^^ defined(_M_ARM64) || defined(_M_ARM64EC) ^^^
Comment thread
StephanTLavavej marked this conversation as resolved.

if constexpr (_Traits::_Vectorized) {
auto _Base = static_cast<const char*>(_First);
size_t _Portion_byte_size = _Byte_length(_First, _Last) & ~_Traits::_Vec_mask;
Expand Down Expand Up @@ -3029,6 +3036,14 @@ namespace {
return _Traits::_H_max_u(_Vals);
}
};
const auto _Less_wrap = [](const auto _Lhs, const auto _Rhs) noexcept {
if constexpr (_Sign || !_Traits::_Has_unsigned_cmp) {
return _Lhs < _Rhs;
} else {
using _UTy = _Traits::_Unsigned_t;
return static_cast<_UTy>(_Lhs) < static_cast<_UTy>(_Rhs);
}
};
#else // ^^^ defined(_M_ARM64) || defined(_M_ARM64EC) / !defined(_M_ARM64) && !defined(_M_ARM64EC) vvv
const auto _Cmp_gt_wrap = [](const auto _First, const auto _Second) noexcept {
return _Traits::_Cmp_gt(_First, _Second);
Expand All @@ -3041,6 +3056,7 @@ namespace {
};
const auto _H_min_wrap = [](const auto _Vals) noexcept { return _Traits::_H_min(_Vals); };
const auto _H_max_wrap = [](const auto _Vals) noexcept { return _Traits::_H_max(_Vals); };
const auto _Less_wrap = [](const auto _Lhs, const auto _Rhs) noexcept { return _Lhs < _Rhs; };
Comment thread
StephanTLavavej marked this conversation as resolved.
#endif // ^^^ !defined(_M_ARM64) && !defined(_M_ARM64EC) ^^^

const auto _Update_min_max = [&](const auto _Cur_vals, [[maybe_unused]] const auto _Blend_idx_0,
Expand Down Expand Up @@ -3134,7 +3150,7 @@ namespace {
const auto _H_min = _H_min_wrap(_Cur_vals_min);
const auto _H_min_val = _Traits::_Get_any(_H_min); // Get any element of it

if (_H_min_val < _Cur_min_val) { // Current horizontal min is less than the old
if (_Less_wrap(_H_min_val, _Cur_min_val)) { // Current horizontal min is less than the old
_Cur_min_val = _H_min_val; // update min
// Mask of all elems eq to min
const auto _Eq_mask = _Traits::_Cmp_eq(_H_min, _Cur_vals_min);
Expand All @@ -3161,8 +3177,8 @@ namespace {
const auto _H_max = _H_max_wrap(_Cur_vals_max);
const auto _H_max_val = _Traits::_Get_any(_H_max); // Get any element of it

if (_Mode == _Mode_both && _Cur_max_val <= _H_max_val
|| _Mode == _Mode_max && _Cur_max_val < _H_max_val) {
if (_Mode == _Mode_both && !_Less_wrap(_H_max_val, _Cur_max_val)
|| _Mode == _Mode_max && _Less_wrap(_Cur_max_val, _H_max_val)) {
// max_element: current horizontal max is greater than the old, update max
// minmax_element: current horizontal max is not less than the old, update max
_Cur_max_val = _H_max_val;
Expand Down
33 changes: 33 additions & 0 deletions tests/std/tests/VSO_0000000_vector_algorithms/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,37 @@ void test_min_max_element_special_cases() {
== v.begin() + 2 * block_size_in_elements + last_vector_first_elem + 9);
}

// GH-6373 ARM64/ARM64EC vectorized minmax_element() family mishandles unsigned elements
void test_gh_6373() {
// These test cases are 16 bytes, to reach the vectorization threshold.

// In these cases, the minimum is 2^(N-1) - 1 and is located after index 0.
test_case_min_max_element(
vector<uint8_t>{155, 153, 248, 150, 189, 140, 247, 178, 244, 164, 226, 214, 239, 215, 176, 127});
test_case_min_max_element(vector<uint16_t>{0x8011, 0x8022, 0x8033, 0x8044, 0x8055, 0x8066, 0x8077, 0x7FFF});
test_case_min_max_element(vector<uint32_t>{0x8000'0011UL, 0x8000'0022UL, 0x8000'0033UL, 0x7FFF'FFFFUL});
test_case_min_max_element(vector<uint64_t>{0x8000'0000'0000'0011ULL, 0x7FFF'FFFF'FFFF'FFFFULL});

// In these cases, the maximum is 2^(N-1) and is located after index 0.
test_case_min_max_element(vector<uint8_t>{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 128});
test_case_min_max_element(vector<uint16_t>{1, 2, 3, 4, 5, 6, 7, 0x8000});
test_case_min_max_element(vector<uint32_t>{1, 2, 3, 0x8000'0000UL});
test_case_min_max_element(vector<uint64_t>{1, 0x8000'0000'0000'0000ULL});

{
// This is an extra test case to exercise an additional failure mode that was resolved by the same fix.
// For uint8_t, _Max_portion_byte_size = _Portion_max * _Vec_size = 256 * 16 = 4096.
// This test case has 4112 = 4096 + 16 bytes, which is a max portion followed by a single vector.
// The scenario is when all of the first 4096 bytes are >= 128, then any of the last 16 bytes are < 128.
vector<uint8_t> v(4112, 200);
v[1729] = 222;
v[3000] = 222;
v[4100] = 11;
v[4105] = 11;
test_case_min_max_element(v);
}
}

template <class T>
void test_is_sorted_until(mt19937_64& gen) {
using Limits = numeric_limits<T>;
Expand Down Expand Up @@ -1325,6 +1356,8 @@ void test_vector_algorithms(mt19937_64& gen) {
test_case_min_max_element(
vector<int64_t>{-6604286336755016904, -4365366089374418225, 6104371530830675888, -8582621853879131834});

test_gh_6373();

test_is_sorted_until<char>(gen);
test_is_sorted_until<signed char>(gen);
test_is_sorted_until<unsigned char>(gen);
Expand Down