Skip to content

V
V8
Commit 9ed9dff9 authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ
Browse files
Options

Revert "[builtins] use SIMD IndexOf/includes on large arrays" 

This reverts commit ab76ffc8.

Reason for revert: Breaks on UBSan: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20UBSan/21444/overview

Original change's description:
> [builtins] use SIMD IndexOf/includes on large arrays
>
> Change-Id: If751e813c7f45a4d18b84e8c0314a54c84894d61
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3639203
> Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#80771}

Change-Id: I62dd4249df122af567680cbaa18deb3c9ed44c90
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3672416
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80773}
parent a7aba02a
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 19 additions and 952 deletions
BUILD.bazel
View file @ 9ed9dff9
......@@ -1819,8 +1819,6 @@ filegroup(
"src/objects/shared-function-info-inl.h",
"src/objects/shared-function-info.cc",
"src/objects/shared-function-info.h",
"src/objects/simd.cc",
"src/objects/simd.h",
"src/objects/slots-atomic-inl.h",
"src/objects/slots-inl.h",
"src/objects/slots.h",
......
BUILD.gn
View file @ 9ed9dff9
......@@ -3315,7 +3315,6 @@ v8_header_set("v8_internal_headers") {
"src/objects/script.h",
"src/objects/shared-function-info-inl.h",
"src/objects/shared-function-info.h",
"src/objects/simd.h",
"src/objects/slots-atomic-inl.h",
"src/objects/slots-inl.h",
"src/objects/slots.h",
......@@ -4456,7 +4455,6 @@ v8_source_set("v8_base_without_compiler") {
"src/objects/property.cc",
"src/objects/scope-info.cc",
"src/objects/shared-function-info.cc",
"src/objects/simd.cc",
"src/objects/source-text-module.cc",
"src/objects/string-comparator.cc",
"src/objects/string-table.cc",
......
src/builtins/builtins-array-gen.cc
View file @ 9ed9dff9
This diff is collapsed.
src/builtins/builtins-definitions.h
View file @ 9ed9dff9
......@@ -364,7 +364,6 @@ namespace internal {
/* ES6 #sec-array.prototype.fill */ \
CPP(ArrayPrototypeFill) \
/* ES7 #sec-array.prototype.includes */ \
TFS(ArrayIncludesSmi, kElements, kSearchElement, kLength, kFromIndex) \
TFS(ArrayIncludesSmiOrObject, kElements, kSearchElement, kLength, \
kFromIndex) \
TFS(ArrayIncludesPackedDoubles, kElements, kSearchElement, kLength, \
......@@ -373,7 +372,6 @@ namespace internal {
kFromIndex) \
TFJ(ArrayIncludes, kDontAdaptArgumentsSentinel) \
/* ES6 #sec-array.prototype.indexof */ \
TFS(ArrayIndexOfSmi, kElements, kSearchElement, kLength, kFromIndex) \
TFS(ArrayIndexOfSmiOrObject, kElements, kSearchElement, kLength, kFromIndex) \
TFS(ArrayIndexOfPackedDoubles, kElements, kSearchElement, kLength, \
kFromIndex) \
......
src/codegen/external-reference.cc
View file @ 9ed9dff9
......@@ -29,7 +29,6 @@
#include "src/objects/object-type.h"
#include "src/objects/objects-inl.h"
#include "src/objects/ordered-hash-table.h"
#include "src/objects/simd.h"
#include "src/regexp/experimental/experimental.h"
#include "src/regexp/regexp-interpreter.h"
#include "src/regexp/regexp-macro-assembler-arch.h"
......@@ -1003,9 +1002,6 @@ FUNCTION_REFERENCE(try_string_to_index_or_lookup_existing,
FUNCTION_REFERENCE(string_from_forward_table,
StringForwardingTable::GetForwardStringAddress)
FUNCTION_REFERENCE(string_to_array_index_function, String::ToArrayIndex)
FUNCTION_REFERENCE(array_indexof_includes_smi_or_object,
ArrayIndexOfIncludesSmiOrObject)
FUNCTION_REFERENCE(array_indexof_includes_double, ArrayIndexOfIncludesDouble)
static Address LexicographicCompareWrapper(Isolate* isolate, Address smi_x,
Address smi_y) {
......
src/codegen/external-reference.h
View file @ 9ed9dff9
......@@ -186,9 +186,6 @@ class StatsCounter;
V(external_two_byte_string_get_chars, "external_two_byte_string_get_chars") \
V(smi_lexicographic_compare_function, "smi_lexicographic_compare_function") \
V(string_to_array_index_function, "String::ToArrayIndex") \
V(array_indexof_includes_smi_or_object, \
"array_indexof_includes_smi_or_object") \
V(array_indexof_includes_double, "array_indexof_includes_double") \
V(try_string_to_index_or_lookup_existing, \
"try_string_to_index_or_lookup_existing") \
V(string_from_forward_table, "string_from_forward_table") \
......
src/compiler/js-call-reducer.cc
View file @ 9ed9dff9
......@@ -7,7 +7,6 @@
#include <functional>
#include "src/api/api-inl.h"
#include "src/base/cpu.h"
#include "src/base/small-vector.h"
#include "src/builtins/builtins-promise.h"
#include "src/builtins/builtins-utils.h"
......@@ -2156,7 +2155,6 @@ Callable GetCallableForArrayIndexOfIncludes(ArrayIndexOfIncludesVariant variant,
switch (elements_kind) {
case PACKED_SMI_ELEMENTS:
case HOLEY_SMI_ELEMENTS:
return Builtins::CallableFor(isolate, Builtin::kArrayIndexOfSmi);
case PACKED_ELEMENTS:
case HOLEY_ELEMENTS:
return Builtins::CallableFor(isolate,
......@@ -2174,7 +2172,6 @@ Callable GetCallableForArrayIndexOfIncludes(ArrayIndexOfIncludesVariant variant,
switch (elements_kind) {
case PACKED_SMI_ELEMENTS:
case HOLEY_SMI_ELEMENTS:
return Builtins::CallableFor(isolate, Builtin::kArrayIncludesSmi);
case PACKED_ELEMENTS:
case HOLEY_ELEMENTS:
return Builtins::CallableFor(isolate,
......@@ -2192,6 +2189,7 @@ Callable GetCallableForArrayIndexOfIncludes(ArrayIndexOfIncludesVariant variant,
}
} // namespace
TNode<Object>
IteratingArrayBuiltinReducerAssembler::ReduceArrayPrototypeIndexOfIncludes(
ElementsKind kind, ArrayIndexOfIncludesVariant variant) {
......@@ -2229,6 +2227,7 @@ IteratingArrayBuiltinReducerAssembler::ReduceArrayPrototypeIndexOfIncludes(
return Call4(GetCallableForArrayIndexOfIncludes(variant, kind, isolate()),
context, elements, search_element, length, from_index);
}
namespace {
struct PromiseCtorFrameStateParams {
......
src/debug/debug-evaluate.cc
View file @ 9ed9dff9
......@@ -1072,11 +1072,9 @@ static bool TransitivelyCalledBuiltinHasNoSideEffect(Builtin caller,
case Builtin::kArrayForEachLoopContinuation:
case Builtin::kArrayIncludesHoleyDoubles:
case Builtin::kArrayIncludesPackedDoubles:
case Builtin::kArrayIncludesSmi:
case Builtin::kArrayIncludesSmiOrObject:
case Builtin::kArrayIndexOfHoleyDoubles:
case Builtin::kArrayIndexOfPackedDoubles:
case Builtin::kArrayIndexOfSmi:
case Builtin::kArrayIndexOfSmiOrObject:
case Builtin::kArrayMapLoopContinuation:
case Builtin::kArrayReduceLoopContinuation:
......
src/objects/simd.cc deleted 100644 → 0
View file @ a7aba02a
This diff is collapsed.
src/objects/simd.h deleted 100644 → 0
View file @ a7aba02a
// Copyright 2022 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef V8_OBJECTS_SIMD_H_
#define V8_OBJECTS_SIMD_H_
#include <cstdint>
#include "include/v8-internal.h"
namespace v8 {
namespace internal {
uintptr_t ArrayIndexOfIncludesSmiOrObject(Address array_start,
uintptr_t array_len,
uintptr_t from_index,
Address search_element);
uintptr_t ArrayIndexOfIncludesDouble(Address array_start, uintptr_t array_len,
uintptr_t from_index,
Address search_element);
} // namespace internal
} // namespace v8
#endif // V8_OBJECTS_SIMD_H_
test/mjsunit/array-includes-simd.js deleted 100644 → 0
View file @ a7aba02a
// Copyright 2022 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Large array of packed Smi, and Smi search_element
(() => {
let a = [];
for (let i = 0; i < 200; i++) {
a[i] = i;
}
function testArrayIncludes(idx) {
return a.includes(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
assertEquals(true, testArrayIncludes(i));
}
// With fromIndex
for (let i = 0, from_index = 0; i+from_index < 200; i += 2, from_index++) {
assertEquals(true, testArrayIncludes(i, from_index));
}
})();
// Large array of holey Smi, and Smi search_element
(() => {
let a = [];
// Skipping every other item when initializing
for (let i = 0; i < 200; i+=2) {
a[i] = i;
}
function testArrayIncludes(idx) {
return a.includes(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
if (i % 2 == 0) {
assertEquals(true, testArrayIncludes(i));
} else {
assertEquals(false, testArrayIncludes(i));
}
}
// With fromIndex
for (let i = 0, from_index = 0; i + from_index < 200; i += 2, from_index++) {
if (i % 2 == 0) {
assertEquals(true, testArrayIncludes(i, from_index));
} else {
assertEquals(false, testArrayIncludes(i, from_index));
}
}
})();
// Large array of packed Doubles, and Double search_element
(() => {
let a = [];
for (let i = 0; i < 200; i++) {
a[i] = i + 0.5;
}
function testArrayIncludes(idx) {
return a.includes(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
assertEquals(true, testArrayIncludes(i + 0.5));
}
// With fromIndex
for (let i = 0, from_index = 0; i+from_index < 200; i += 2, from_index++) {
assertEquals(true, testArrayIncludes(i+0.5, from_index));
}
})();
// Large array of holey Doubles, and Double search_element
(() => {
let a = [];
// Skipping every other item when initializing
for (let i = 0; i < 200; i+=2) {
a[i] = i + 0.5;
}
function testArrayIncludes(idx) {
return a.includes(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
if (i % 2 == 0) {
assertEquals(true, testArrayIncludes(i + 0.5));
} else {
assertEquals(false, testArrayIncludes(i + 0.5));
}
}
// With fromIndex
for (let i = 0, from_index = 0; i + from_index < 200; i += 2, from_index++) {
if (i % 2 == 0) {
assertEquals(true, testArrayIncludes(i+0.5, from_index));
} else {
assertEquals(false, testArrayIncludes(i+0.5, from_index));
}
}
})();
// Large array of packed objects, and object search_element
(() => {
let a = [];
let b = [];
for (let i = 0; i < 200; i++) {
a[i] = { v: i };
b[i] = a[i];
}
function testArrayIncludes(idx) {
return a.includes(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
assertEquals(true, testArrayIncludes(b[i]));
}
// With fromIndex
for (let i = 0, from_index = 0; i+from_index < 200; i += 2, from_index++) {
assertEquals(true, testArrayIncludes(b[i], from_index));
}
})();
// Large array of holey objects, and object search_element
(() => {
let a = [];
let b = [];
// Skipping every other item when initializing
for (let i = 0; i < 200; i++) {
b[i] = { v: i };
if (i % 2 == 0) {
a[i] = b[i];
}
}
function testArrayIncludes(idx) {
return a.includes(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
if (i % 2 == 0) {
assertEquals(true, testArrayIncludes(b[i]));
} else {
assertEquals(false, testArrayIncludes(b[i]));
}
}
// With fromIndex
for (let i = 0, from_index = 0; i + from_index < 200; i += 2, from_index++) {
if (i % 2 == 0) {
assertEquals(true, testArrayIncludes(b[i], from_index));
} else {
assertEquals(false, testArrayIncludes(b[i], from_index));
}
}
})();
test/mjsunit/array-indexof-simd.js deleted 100644 → 0
View file @ a7aba02a
// Copyright 2022 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Large array of packed Smi, and Smi search_element
(() => {
let a = [];
for (let i = 0; i < 200; i++) {
a[i] = i;
}
function testArrayIndexOf(idx) {
return a.indexOf(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
assertEquals(i, testArrayIndexOf(i));
}
// With fromIndex
for (let i = 0, from_index = 0; i+from_index < 200; i += 2, from_index++) {
assertEquals(i, testArrayIndexOf(i, from_index));
}
})();
// Large array of holey Smi, and Smi search_element
(() => {
let a = [];
// Skipping every other item when initializing
for (let i = 0; i < 200; i+=2) {
a[i] = i;
}
function testArrayIndexOf(idx) {
return a.indexOf(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
if (i % 2 == 0) {
assertEquals(i, testArrayIndexOf(i));
} else {
assertEquals(-1, testArrayIndexOf(i));
}
}
// With fromIndex
for (let i = 0, from_index = 0; i + from_index < 200; i += 2, from_index++) {
if (i % 2 == 0) {
assertEquals(i, testArrayIndexOf(i, from_index));
} else {
assertEquals(-1, testArrayIndexOf(i, from_index));
}
}
})();
// Large array of packed Doubles, and Double search_element
(() => {
let a = [];
for (let i = 0; i < 200; i++) {
a[i] = i + 0.5;
}
function testArrayIndexOf(idx) {
return a.indexOf(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
assertEquals(i, testArrayIndexOf(i + 0.5));
}
// With fromIndex
for (let i = 0, from_index = 0; i+from_index < 200; i += 2, from_index++) {
assertEquals(i, testArrayIndexOf(i+0.5, from_index));
}
})();
// Large array of holey Doubles, and Double search_element
(() => {
let a = [];
// Skipping every other item when initializing
for (let i = 0; i < 200; i+=2) {
a[i] = i + 0.5;
}
function testArrayIndexOf(idx) {
return a.indexOf(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
if (i % 2 == 0) {
assertEquals(i, testArrayIndexOf(i + 0.5));
} else {
assertEquals(-1, testArrayIndexOf(i + 0.5));
}
}
// With fromIndex
for (let i = 0, from_index = 0; i + from_index < 200; i += 2, from_index++) {
if (i % 2 == 0) {
assertEquals(i, testArrayIndexOf(i+0.5, from_index));
} else {
assertEquals(-1, testArrayIndexOf(i+0.5, from_index));
}
}
})();
// Large array of packed objects, and object search_element
(() => {
let a = [];
let b = [];
for (let i = 0; i < 200; i++) {
a[i] = { v: i };
b[i] = a[i];
}
function testArrayIndexOf(idx) {
return a.indexOf(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
assertEquals(i, testArrayIndexOf(b[i]));
}
// With fromIndex
for (let i = 0, from_index = 0; i+from_index < 200; i += 2, from_index++) {
assertEquals(i, testArrayIndexOf(b[i], from_index));
}
})();
// Large array of holey objects, and object search_element
(() => {
let a = [];
let b = [];
// Skipping every other item when initializing
for (let i = 0; i < 200; i++) {
b[i] = { v: i };
if (i % 2 == 0) {
a[i] = b[i];
}
}
function testArrayIndexOf(idx) {
return a.indexOf(idx);
}
// Without fromIndex
for (let i = 0; i < 200; i++) {
if (i % 2 == 0) {
assertEquals(i, testArrayIndexOf(b[i]));
} else {
assertEquals(-1, testArrayIndexOf(b[i]));
}
}
// With fromIndex
for (let i = 0, from_index = 0; i + from_index < 200; i += 2, from_index++) {
if (i % 2 == 0) {
assertEquals(i, testArrayIndexOf(b[i], from_index));
} else {
assertEquals(-1, testArrayIndexOf(b[i], from_index));
}
}
})();
// This test checks that when the item that IndexOf searches is present multiple
// time, the correct index is returned (in particular, when a single SIMD vector
// had multiple matches). For instance, if we do:
//
// [1, 2, 1, 3].indexOf(1)
//
// Then it should return 0 rather than 2.
(() => {
// The patterns that this function will check, where for instance patternABAB
// means that we'd like to build a vector containing {A, B, A, B}.
let patterns = {
patternABAB : (a, b, c, d) => [a, b, a, b, c, d, c, d],
patternAABB : (a, b, c, d) => [a, a, b, b, c, c, d, d],
patternABBA : (a, b, c, d) => [a, b, b, a, c, d, d, c],
patternABAA : (a, b, c, d) => [a, b, a, a, c, d, c, c],
patternAABA : (a, b, c, d) => [a, a, b, a, c, c, d, c],
patternAAAB : (a, b, c, d) => [a, a, a, b, c, c, c, d],
patternBAAA : (a, b, c, d) => [b, a, a, a, d, c, c, c]
};
// Starting |a| with a bunch of 0s, which might be handled by the scalar loop
// that the SIMD code does to reach 16/32-byte alignment.
let a = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
let next_int = 1;
for (const [_, pattern] of Object.entries(patterns)) {
// It's a bit tricky to ensure that 2 items will be in the same SIMD batch
// because we can't control the alignment of the array from JS, and the
// SIMD code will start by skipping the first items to have the memory
// aligned on 16/32 bytes. So, we put each pattern 8 times in a row in |a|,
// but each time with an additional item, to make sure that each of those 8
// repeated pattern have a different alignment.
for (let i = 0; i < 8; i++) {
a = a.concat(pattern(next_int, next_int + 1, next_int + 2, next_int + 3));
a.push(next_int + 4); // By adding a 9th item, we make sure that the
// alignment of the next batch is not the same as
// the current one.
next_int += 5;
}
}
let b = a.slice();
b[10000] = 42; // Switch b to dictionary mode so that the SIMD code won't be
// used for it. We can then use `b.indexOf` as reference.
for (let x of b) {
if (x == undefined) break;
assertEquals(b.indexOf(x), a.indexOf(x));
}
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment