Skip to content
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.

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

Fix silent bad codegen for vectorized meow_element() above 4 GB #3619

Merged
merged 6 commits into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions stl/src/vector_algorithms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_Res._Min = _Base + _V_pos * 16 + _H_pos; // Finally, compute the pointer
_Res._Min =
_Base + static_cast<size_t>(_V_pos) * 16 + _H_pos; // Finally, compute the pointer
strega-nil-ms marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -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<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
Expand Down
111 changes: 111 additions & 0 deletions tests/std/include/test_min_max_element_support.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#pragma once

#include <algorithm>
#include <cassert>
#include <cstddef>
#include <utility>
#include <vector>

#ifdef __cpp_lib_concepts
#include <ranges>
#endif

template <class FwdIt>
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 <class FwdIt>
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 <class FwdIt>
std::pair<FwdIt, FwdIt> last_known_good_minmax_element(FwdIt first, FwdIt last) {
// find smallest and largest elements
std::pair<FwdIt, FwdIt> 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 <class T>
void test_case_min_max_element(const std::vector<T>& 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<ptrdiff_t>(input.size())));
auto actual_max_sized_range = std::ranges::max_element(take(input, static_cast<ptrdiff_t>(input.size())));
auto actual_minmax_sized_range = std::ranges::minmax_element(take(input, static_cast<ptrdiff_t>(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
}
1 change: 1 addition & 0 deletions tests/std/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions tests/std/tests/GH_003617_vectorized_meow_element/env.lst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\fast_matrix.lst
44 changes: 44 additions & 0 deletions tests/std/tests/GH_003617_vectorized_meow_element/test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

// REQUIRES: x64

#ifdef _M_X64

#include <cstddef>
#include <isa_availability.h>
#include <vector>

#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<unsigned long>(isa));
}

void test_gh_3617() {
// Test GH-3617 "<algorithm>: Silent bad codegen for vectorized meow_element() above 4 GB".
constexpr size_t n = 0x4000'0010;

vector<int> 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 ^^^
103 changes: 6 additions & 97 deletions tests/std/tests/VSO_0000000_vector_algorithms/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include <ranges>
#endif

#include "test_min_max_element_support.hpp"

using namespace std;

#pragma warning(disable : 4984) // 'if constexpr' is a C++17 language extension
Expand Down Expand Up @@ -121,103 +123,6 @@ void test_find(mt19937_64& gen) {
}
}

template <class FwdIt>
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 <class FwdIt>
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 <class FwdIt>
pair<FwdIt, FwdIt> last_known_good_minmax_element(FwdIt first, FwdIt last) {
// find smallest and largest elements
pair<FwdIt, FwdIt> 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 <class T>
void test_case_min_max_element(const vector<T>& 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<ptrdiff_t>(input.size())));
auto actual_max_sized_range = ranges::max_element(take(input, static_cast<ptrdiff_t>(input.size())));
auto actual_minmax_sized_range = ranges::minmax_element(take(input, static_cast<ptrdiff_t>(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 <class T>
void test_min_max_element(mt19937_64& gen) {
using Limits = numeric_limits<T>;
Expand Down Expand Up @@ -504,12 +409,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
}