Skip to content

Commit

Permalink
Fix silent bad codegen for vectorized meow_element() above 4 GB (#3619
Browse files Browse the repository at this point in the history
)
  • Loading branch information
StephanTLavavej authored Apr 7, 2023
1 parent adaf68c commit cb86d7e
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 99 deletions.
6 changes: 4 additions & 2 deletions stl/src/vector_algorithms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,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<size_t>(_V_pos) * 16 + _H_pos; // Finally, compute the pointer
}
}

Expand Down Expand Up @@ -925,7 +926,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
}

0 comments on commit cb86d7e

Please sign in to comment.