[gasm] Fix deopt frame state in Array.p.reduce and reduceRight
This fixes a bug in lazy deopts caused by calls to the callback function in Array.prototype.reduce and reduceRight. The deopt continuation expects the *next* iteration's index value but we actually passed the current iteration's value. The user-visible effect of this bug was that sometimes, an unexpected additional call to the callback function would occur. It was introduced by https://crrev.com/c/1934329. Bug: v8:9972,chromium:1049982 Change-Id: Icfd2ef076209e20602f54d4662220e1d4c5d07ee Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2049850 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#66226}
This commit is contained in:
parent
89b248b6f8
commit
099de337fe
@ -1314,11 +1314,12 @@ TNode<Object> IteratingArrayBuiltinReducerAssembler::ReduceArrayPrototypeReduce(
|
|||||||
element =
|
element =
|
||||||
MaybeSkipHole(element, kind, &continue_label, *accumulator);
|
MaybeSkipHole(element, kind, &continue_label, *accumulator);
|
||||||
|
|
||||||
|
TNode<Number> next_k = step(k);
|
||||||
TNode<Object> next_accumulator = JSCall4(
|
TNode<Object> next_accumulator = JSCall4(
|
||||||
fncallback, UndefinedConstant(), *accumulator, element, k,
|
fncallback, UndefinedConstant(), *accumulator, element, k,
|
||||||
receiver,
|
receiver,
|
||||||
ReduceLoopLazyFrameState(frame_state_params, receiver,
|
ReduceLoopLazyFrameState(frame_state_params, receiver,
|
||||||
fncallback, k, original_length));
|
fncallback, next_k, original_length));
|
||||||
Goto(&continue_label, next_accumulator);
|
Goto(&continue_label, next_accumulator);
|
||||||
|
|
||||||
Bind(&continue_label);
|
Bind(&continue_label);
|
||||||
|
@ -364,6 +364,8 @@
|
|||||||
'compiler/serializer-dead-after-jump': [SKIP],
|
'compiler/serializer-dead-after-jump': [SKIP],
|
||||||
'compiler/serializer-dead-after-return': [SKIP],
|
'compiler/serializer-dead-after-return': [SKIP],
|
||||||
'compiler/serializer-transition-propagation': [SKIP],
|
'compiler/serializer-transition-propagation': [SKIP],
|
||||||
|
'regress/regress-1049982-1': [SKIP],
|
||||||
|
'regress/regress-1049982-2': [SKIP],
|
||||||
|
|
||||||
# These tests check that we can trace the compiler.
|
# These tests check that we can trace the compiler.
|
||||||
'tools/compiler-trace-flags': [SKIP],
|
'tools/compiler-trace-flags': [SKIP],
|
||||||
@ -1028,6 +1030,13 @@
|
|||||||
'wasm/grow-memory': [SKIP],
|
'wasm/grow-memory': [SKIP],
|
||||||
}], # variant == nooptimization and (arch == arm or arch == arm64) and simulator_run
|
}], # variant == nooptimization and (arch == arm or arch == arm64) and simulator_run
|
||||||
|
|
||||||
|
##############################################################################
|
||||||
|
['variant == nooptimization', {
|
||||||
|
# Tests that depend on optimization (beyond doing assertOptimized).
|
||||||
|
'regress/regress-1049982-1': [SKIP],
|
||||||
|
'regress/regress-1049982-2': [SKIP],
|
||||||
|
}], # variant == nooptimization
|
||||||
|
|
||||||
##############################################################################
|
##############################################################################
|
||||||
['(arch == arm or arch == arm64)', {
|
['(arch == arm or arch == arm64)', {
|
||||||
# Flaky tests: https://crbug.com/v8/8090
|
# Flaky tests: https://crbug.com/v8/8090
|
||||||
@ -1118,6 +1127,8 @@
|
|||||||
|
|
||||||
# Some tests rely on inlining.
|
# Some tests rely on inlining.
|
||||||
'compiler/opt-higher-order-functions': [SKIP],
|
'compiler/opt-higher-order-functions': [SKIP],
|
||||||
|
'regress/regress-1049982-1': [SKIP],
|
||||||
|
'regress/regress-1049982-2': [SKIP],
|
||||||
}], # variant == turboprop
|
}], # variant == turboprop
|
||||||
|
|
||||||
##############################################################################
|
##############################################################################
|
||||||
|
31
test/mjsunit/regress/regress-1049982-1.js
Normal file
31
test/mjsunit/regress/regress-1049982-1.js
Normal file
@ -0,0 +1,31 @@
|
|||||||
|
// Copyright 2020 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.
|
||||||
|
//
|
||||||
|
// Flags: --allow-natives-syntax
|
||||||
|
|
||||||
|
const xs = [1,2,3,4,5,6,7,8,9];
|
||||||
|
let deopt = false;
|
||||||
|
|
||||||
|
function g(acc, x, i) {
|
||||||
|
if (deopt) {
|
||||||
|
assertFalse(%IsBeingInterpreted());
|
||||||
|
Array.prototype.x = 42; // Trigger a lazy deopt.
|
||||||
|
deopt = false;
|
||||||
|
}
|
||||||
|
return acc + x;
|
||||||
|
}
|
||||||
|
|
||||||
|
function f() {
|
||||||
|
return xs.reduce(g, 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
%PrepareFunctionForOptimization(f);
|
||||||
|
%PrepareFunctionForOptimization(g);
|
||||||
|
assertEquals(45, f());
|
||||||
|
assertEquals(45, f());
|
||||||
|
%OptimizeFunctionOnNextCall(f);
|
||||||
|
|
||||||
|
deopt = true;
|
||||||
|
assertEquals(45, f());
|
||||||
|
assertEquals(45, f());
|
31
test/mjsunit/regress/regress-1049982-2.js
Normal file
31
test/mjsunit/regress/regress-1049982-2.js
Normal file
@ -0,0 +1,31 @@
|
|||||||
|
// Copyright 2020 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.
|
||||||
|
//
|
||||||
|
// Flags: --allow-natives-syntax
|
||||||
|
|
||||||
|
const xs = [1,2,3,4,5,6,7,8,9];
|
||||||
|
let deopt = false;
|
||||||
|
|
||||||
|
function g(acc, x, i) {
|
||||||
|
if (deopt) {
|
||||||
|
assertFalse(%IsBeingInterpreted());
|
||||||
|
Array.prototype.x = 42; // Trigger a lazy deopt.
|
||||||
|
deopt = false;
|
||||||
|
}
|
||||||
|
return acc + x;
|
||||||
|
}
|
||||||
|
|
||||||
|
function f() {
|
||||||
|
return xs.reduceRight(g, 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
%PrepareFunctionForOptimization(f);
|
||||||
|
%PrepareFunctionForOptimization(g);
|
||||||
|
assertEquals(45, f());
|
||||||
|
assertEquals(45, f());
|
||||||
|
%OptimizeFunctionOnNextCall(f);
|
||||||
|
|
||||||
|
deopt = true;
|
||||||
|
assertEquals(45, f());
|
||||||
|
assertEquals(45, f());
|
Loading…
Reference in New Issue
Block a user