From 6efa36f6f22989711ab669d605cd4147e24c9a5f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 3 Apr 2023 12:11:05 -0700 Subject: [PATCH 1/6] Fix truncation by adding `static_cast`. --- stl/src/vector_algorithms.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/stl/src/vector_algorithms.cpp b/stl/src/vector_algorithms.cpp index a38d93c281f..6cadbadb6a1 100644 --- a/stl/src/vector_algorithms.cpp +++ b/stl/src/vector_algorithms.cpp @@ -884,7 +884,8 @@ namespace { _BitScanForward(&_H_pos, _Mask); // lgtm [cpp/conditionallyuninitializedvariable] const auto _V_pos = _Traits::_Get_v_pos(_Cur_idx_min, _H_pos); // Extract its vertical index - _Res._Min = _Base + _V_pos * 16 + _H_pos; // Finally, compute the pointer + _Res._Min = + _Base + static_cast(_V_pos) * 16 + _H_pos; // Finally, compute the pointer } } @@ -930,7 +931,8 @@ namespace { } const auto _V_pos = _Traits::_Get_v_pos(_Cur_idx_max, _H_pos); // Extract its vertical index - _Res._Max = _Base + _V_pos * 16 + _H_pos; // Finally, compute the pointer + _Res._Max = + _Base + static_cast(_V_pos) * 16 + _H_pos; // Finally, compute the pointer } } // Horizontal part done, results are saved, now need to see if there is another portion to process From 6c59d2cf935431172d2cfca120eab2d0411239f1 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 3 Apr 2023 12:16:58 -0700 Subject: [PATCH 2/6] Pre-existing: `test_various_containers` as we disable instructions. This was an oversight in GH 804 on 2020-07-27. --- tests/std/tests/VSO_0000000_vector_algorithms/test.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp b/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp index 6b607b762a9..192b5a152b7 100644 --- a/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp +++ b/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp @@ -504,12 +504,16 @@ int main() { #if defined(_M_IX86) || defined(_M_X64) disable_instructions(__ISA_AVAILABLE_AVX2); test_vector_algorithms(gen); + test_various_containers(); + disable_instructions(__ISA_AVAILABLE_SSE42); test_vector_algorithms(gen); + test_various_containers(); #endif // defined(_M_IX86) || defined(_M_X64) #if defined(_M_IX86) disable_instructions(__ISA_AVAILABLE_SSE2); test_vector_algorithms(gen); + test_various_containers(); #endif // defined(_M_IX86) #endif // _M_CEE_PURE } From 8cccf62bb19cc63f2cb22c530f4c6cd214a8ee7d Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 3 Apr 2023 13:44:52 -0700 Subject: [PATCH 3/6] Extract test_min_max_element_support.hpp. --- .../include/test_min_max_element_support.hpp | 111 ++++++++++++++++++ .../VSO_0000000_vector_algorithms/test.cpp | 99 +--------------- 2 files changed, 113 insertions(+), 97 deletions(-) create mode 100644 tests/std/include/test_min_max_element_support.hpp diff --git a/tests/std/include/test_min_max_element_support.hpp b/tests/std/include/test_min_max_element_support.hpp new file mode 100644 index 00000000000..2d764b3dbf0 --- /dev/null +++ b/tests/std/include/test_min_max_element_support.hpp @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#pragma once + +#include +#include +#include +#include +#include + +#ifdef __cpp_lib_concepts +#include +#endif + +template +FwdIt last_known_good_min_element(FwdIt first, FwdIt last) { + FwdIt result = first; + + for (; first != last; ++first) { + if (*first < *result) { + result = first; + } + } + + return result; +} + +template +FwdIt last_known_good_max_element(FwdIt first, FwdIt last) { + FwdIt result = first; + + for (; first != last; ++first) { + if (*result < *first) { + result = first; + } + } + + return result; +} + +template +std::pair last_known_good_minmax_element(FwdIt first, FwdIt last) { + // find smallest and largest elements + std::pair found(first, first); + + if (first != last) { + while (++first != last) { // process one or two elements + FwdIt next = first; + if (++next == last) { // process last element + if (*first < *found.first) { + found.first = first; + } else if (!(*first < *found.second)) { + found.second = first; + } + } else { // process next two elements + if (*next < *first) { // test next for new smallest + if (*next < *found.first) { + found.first = next; + } + + if (!(*first < *found.second)) { + found.second = first; + } + } else { // test first for new smallest + if (*first < *found.first) { + found.first = first; + } + + if (!(*next < *found.second)) { + found.second = next; + } + } + first = next; + } + } + } + + return found; +} + +template +void test_case_min_max_element(const std::vector& input) { + auto expected_min = last_known_good_min_element(input.begin(), input.end()); + auto expected_max = last_known_good_max_element(input.begin(), input.end()); + auto expected_minmax = last_known_good_minmax_element(input.begin(), input.end()); + auto actual_min = std::min_element(input.begin(), input.end()); + auto actual_max = std::max_element(input.begin(), input.end()); + auto actual_minmax = std::minmax_element(input.begin(), input.end()); + assert(expected_min == actual_min); + assert(expected_max == actual_max); + assert(expected_minmax == actual_minmax); +#ifdef __cpp_lib_concepts + using std::ranges::views::take, std::ptrdiff_t; + + auto actual_min_range = std::ranges::min_element(input); + auto actual_max_range = std::ranges::max_element(input); + auto actual_minmax_range = std::ranges::minmax_element(input); + auto actual_min_sized_range = std::ranges::min_element(take(input, static_cast(input.size()))); + auto actual_max_sized_range = std::ranges::max_element(take(input, static_cast(input.size()))); + auto actual_minmax_sized_range = std::ranges::minmax_element(take(input, static_cast(input.size()))); + assert(expected_min == actual_min_range); + assert(expected_max == actual_max_range); + assert(expected_minmax.first == actual_minmax_range.min); + assert(expected_minmax.second == actual_minmax_range.max); + assert(expected_min == actual_min_sized_range); + assert(expected_max == actual_max_sized_range); + assert(expected_minmax.first == actual_minmax_sized_range.min); + assert(expected_minmax.second == actual_minmax_sized_range.max); +#endif // __cpp_lib_concepts +} diff --git a/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp b/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp index 192b5a152b7..22fa2b285ea 100644 --- a/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp +++ b/tests/std/tests/VSO_0000000_vector_algorithms/test.cpp @@ -19,6 +19,8 @@ #include #endif +#include "test_min_max_element_support.hpp" + using namespace std; #pragma warning(disable : 4984) // 'if constexpr' is a C++17 language extension @@ -121,103 +123,6 @@ void test_find(mt19937_64& gen) { } } -template -FwdIt last_known_good_min_element(FwdIt first, FwdIt last) { - FwdIt result = first; - - for (; first != last; ++first) { - if (*first < *result) { - result = first; - } - } - - return result; -} - -template -FwdIt last_known_good_max_element(FwdIt first, FwdIt last) { - FwdIt result = first; - - for (; first != last; ++first) { - if (*result < *first) { - result = first; - } - } - - return result; -} - -template -pair last_known_good_minmax_element(FwdIt first, FwdIt last) { - // find smallest and largest elements - pair found(first, first); - - if (first != last) { - while (++first != last) { // process one or two elements - FwdIt next = first; - if (++next == last) { // process last element - if (*first < *found.first) { - found.first = first; - } else if (!(*first < *found.second)) { - found.second = first; - } - } else { // process next two elements - if (*next < *first) { // test next for new smallest - if (*next < *found.first) { - found.first = next; - } - - if (!(*first < *found.second)) { - found.second = first; - } - } else { // test first for new smallest - if (*first < *found.first) { - found.first = first; - } - - if (!(*next < *found.second)) { - found.second = next; - } - } - first = next; - } - } - } - - return found; -} - -template -void test_case_min_max_element(const vector& input) { - auto expected_min = last_known_good_min_element(input.begin(), input.end()); - auto expected_max = last_known_good_max_element(input.begin(), input.end()); - auto expected_minmax = last_known_good_minmax_element(input.begin(), input.end()); - auto actual_min = min_element(input.begin(), input.end()); - auto actual_max = max_element(input.begin(), input.end()); - auto actual_minmax = minmax_element(input.begin(), input.end()); - assert(expected_min == actual_min); - assert(expected_max == actual_max); - assert(expected_minmax == actual_minmax); -#ifdef __cpp_lib_concepts - using ranges::views::take; - - auto actual_min_range = ranges::min_element(input); - auto actual_max_range = ranges::max_element(input); - auto actual_minmax_range = ranges::minmax_element(input); - auto actual_min_sized_range = ranges::min_element(take(input, static_cast(input.size()))); - auto actual_max_sized_range = ranges::max_element(take(input, static_cast(input.size()))); - auto actual_minmax_sized_range = ranges::minmax_element(take(input, static_cast(input.size()))); - assert(expected_min == actual_min_range); - assert(expected_max == actual_max_range); - assert(expected_minmax.first == actual_minmax_range.min); - assert(expected_minmax.second == actual_minmax_range.max); - assert(expected_min == actual_min_sized_range); - assert(expected_max == actual_max_sized_range); - assert(expected_minmax.first == actual_minmax_sized_range.min); - assert(expected_minmax.second == actual_minmax_sized_range.max); -#endif // __cpp_lib_concepts -} - template void test_min_max_element(mt19937_64& gen) { using Limits = numeric_limits; From daf094bbd0e7e3fb41cdda022a0b07101c2c78fb Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 3 Apr 2023 14:12:36 -0700 Subject: [PATCH 4/6] Add GH_003617_vectorized_meow_element. --- tests/std/test.lst | 1 + .../GH_003617_vectorized_meow_element/env.lst | 4 ++ .../test.cpp | 44 +++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 tests/std/tests/GH_003617_vectorized_meow_element/env.lst create mode 100644 tests/std/tests/GH_003617_vectorized_meow_element/test.cpp diff --git a/tests/std/test.lst b/tests/std/test.lst index ea1f5fda4dd..4915e01ab03 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -220,6 +220,7 @@ tests\GH_003022_substr_allocator tests\GH_003105_piecewise_densities tests\GH_003119_error_category_ctor tests\GH_003246_cmath_narrowing +tests\GH_003617_vectorized_meow_element tests\LWG2597_complex_branch_cut tests\LWG3018_shared_ptr_function tests\LWG3121_constrained_tuple_forwarding_ctor diff --git a/tests/std/tests/GH_003617_vectorized_meow_element/env.lst b/tests/std/tests/GH_003617_vectorized_meow_element/env.lst new file mode 100644 index 00000000000..288bc01fbe0 --- /dev/null +++ b/tests/std/tests/GH_003617_vectorized_meow_element/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\fast_matrix.lst diff --git a/tests/std/tests/GH_003617_vectorized_meow_element/test.cpp b/tests/std/tests/GH_003617_vectorized_meow_element/test.cpp new file mode 100644 index 00000000000..5d2378150b5 --- /dev/null +++ b/tests/std/tests/GH_003617_vectorized_meow_element/test.cpp @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +// REQUIRES: x64 + +#ifdef _M_X64 + +#include +#include +#include + +#include "test_min_max_element_support.hpp" + +using namespace std; + +extern "C" long __isa_enabled; + +void disable_instructions(ISA_AVAILABILITY isa) { + __isa_enabled &= ~(1UL << static_cast(isa)); +} + +void test_gh_3617() { + // Test GH-3617 ": Silent bad codegen for vectorized meow_element() above 4 GB". + constexpr size_t n = 0x4000'0010; + + vector v(n, 25); + v[n - 2] = 24; + v[n - 1] = 26; + + test_case_min_max_element(v); +} + +int main() { + test_gh_3617(); + + disable_instructions(__ISA_AVAILABLE_AVX2); + test_gh_3617(); + + disable_instructions(__ISA_AVAILABLE_SSE42); + test_gh_3617(); +} +#else // ^^^ x64 / other architectures vvv +int main() {} +#endif // ^^^ other architectures ^^^ From 3589b283a661e33b199d3a867b3bd97038c47790 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 4 Apr 2023 00:57:29 -0700 Subject: [PATCH 5/6] Use `size_t{_V_pos}`. Co-authored-by: Alex Guteniev --- stl/src/vector_algorithms.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/stl/src/vector_algorithms.cpp b/stl/src/vector_algorithms.cpp index 6cadbadb6a1..ab57b59c7a3 100644 --- a/stl/src/vector_algorithms.cpp +++ b/stl/src/vector_algorithms.cpp @@ -884,8 +884,7 @@ namespace { _BitScanForward(&_H_pos, _Mask); // lgtm [cpp/conditionallyuninitializedvariable] const auto _V_pos = _Traits::_Get_v_pos(_Cur_idx_min, _H_pos); // Extract its vertical index - _Res._Min = - _Base + static_cast(_V_pos) * 16 + _H_pos; // Finally, compute the pointer + _Res._Min = _Base + size_t{_V_pos} * 16 + _H_pos; // Finally, compute the pointer } } @@ -931,8 +930,7 @@ namespace { } const auto _V_pos = _Traits::_Get_v_pos(_Cur_idx_max, _H_pos); // Extract its vertical index - _Res._Max = - _Base + static_cast(_V_pos) * 16 + _H_pos; // Finally, compute the pointer + _Res._Max = _Base + size_t{_V_pos} * 16 + _H_pos; // Finally, compute the pointer } } // Horizontal part done, results are saved, now need to see if there is another portion to process From ccba0455fa36e3b280e455f0bd6ffa4dc7bfbdd0 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 4 Apr 2023 01:33:03 -0700 Subject: [PATCH 6/6] Revert "Use `size_t{_V_pos}`." This reverts commit 3589b283a661e33b199d3a867b3bd97038c47790. --- stl/src/vector_algorithms.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/stl/src/vector_algorithms.cpp b/stl/src/vector_algorithms.cpp index ab57b59c7a3..6cadbadb6a1 100644 --- a/stl/src/vector_algorithms.cpp +++ b/stl/src/vector_algorithms.cpp @@ -884,7 +884,8 @@ namespace { _BitScanForward(&_H_pos, _Mask); // lgtm [cpp/conditionallyuninitializedvariable] const auto _V_pos = _Traits::_Get_v_pos(_Cur_idx_min, _H_pos); // Extract its vertical index - _Res._Min = _Base + size_t{_V_pos} * 16 + _H_pos; // Finally, compute the pointer + _Res._Min = + _Base + static_cast(_V_pos) * 16 + _H_pos; // Finally, compute the pointer } } @@ -930,7 +931,8 @@ namespace { } const auto _V_pos = _Traits::_Get_v_pos(_Cur_idx_max, _H_pos); // Extract its vertical index - _Res._Max = _Base + size_t{_V_pos} * 16 + _H_pos; // Finally, compute the pointer + _Res._Max = + _Base + static_cast(_V_pos) * 16 + _H_pos; // Finally, compute the pointer } } // Horizontal part done, results are saved, now need to see if there is another portion to process