Reland "[maglev] Prevent lazy deopts during maglev's JumpLoop (=OSR)"
This is a reland of commit5d95bd39ca
Original change's description: > [maglev] Prevent lazy deopts during maglev's JumpLoop (=OSR) > > The problem was that synchronous Maglev OSR potentially caused > code deoptimization during compilation dependency finalization; this > led to a lazy deopt when returning from the call to > Runtime_CompileOptimizedOSRFromMaglev. However, a lazy deopt is > disallowed at this point, since a) Maglev doesn't support marking an opcode as both lazy- and eager deopt, and b) the JumpLoop opcode > is already marked as eager deopt since that's how OSR is implemented > under the hood. See also the comment in runtime-compiler.cc. > > We fix this by changing synchronous Maglev-to-Turbofan OSR > behavior s.t. actual OSR compilation is triggered from Ignition > (and not from Maglev). In other words, when synchronous OSR is > requested: > > 1. trigger an eager deopt from Maglev to Ignition by returning a > non-null code object from Runtime_CompileOptimizedOSRFromMaglev. > 2. Ignition handles the pending OSR compile request (through > osr_urgency). > > This CL also reverts previous partial fixes: > > This reverts commit21969e8e24
. > This reverts commit6bcbcfed5c
. > > Bug: chromium:1394279,v8:13585,v8:7700 > Change-Id: I3d64aa39575ad806ba2623102092176ca160ef0b > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4110740 > Commit-Queue: Jakob Linke <jgruber@chromium.org> > Reviewed-by: Victor Gomes <victorgomes@chromium.org> > Cr-Commit-Position: refs/heads/main@{#84922} Bug: chromium:1394279,v8:13585,v8:7700 Change-Id: Id9d1a1ab2dc36e481287a1a25863b45bf281920c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4110746 Commit-Queue: Jakob Linke <jgruber@chromium.org> Auto-Submit: Jakob Linke <jgruber@chromium.org> Commit-Queue: Victor Gomes <victorgomes@chromium.org> Reviewed-by: Victor Gomes <victorgomes@chromium.org> Cr-Commit-Position: refs/heads/main@{#84928}
This commit is contained in:
parent
ba056a8c98
commit
de5df9aebc
@ -3010,9 +3010,10 @@ void AttemptOnStackReplacement(MaglevAssembler* masm,
|
||||
// Two cases may cause us to attempt OSR, in the following order:
|
||||
//
|
||||
// 1) Presence of cached OSR Turbofan code.
|
||||
// 2) The OSR urgency exceeds the current loop depth - in that case, trigger
|
||||
// a Turbofan OSR compilation if in concurrent mode.
|
||||
// Otherwise trigger an eager deopt and delegate OSR to Ignition.
|
||||
// 2) The OSR urgency exceeds the current loop depth - in that case, call
|
||||
// into runtime to trigger a Turbofan OSR compilation. A non-zero return
|
||||
// value means we should deopt into Ignition which will handle all further
|
||||
// necessary steps (rewriting the stack frame, jumping to OSR'd code).
|
||||
//
|
||||
// See also: InterpreterAssembler::OnStackReplacement.
|
||||
|
||||
|
@ -548,18 +548,27 @@ RUNTIME_FUNCTION(Runtime_CompileOptimizedOSRFromMaglev) {
|
||||
DCHECK_EQ(frame->LookupCodeT().kind(), CodeKind::MAGLEV);
|
||||
Handle<JSFunction> function = handle(frame->function(), isolate);
|
||||
|
||||
if (V8_UNLIKELY(function->ActiveTierIsTurbofan())) {
|
||||
// This path is only relevant for tests (all production configurations enable
|
||||
// concurrent OSR). It's quite subtle, if interested read on:
|
||||
if (V8_UNLIKELY(!isolate->concurrent_recompilation_enabled() ||
|
||||
!v8_flags.concurrent_osr)) {
|
||||
// - Synchronous Turbofan compilation may trigger lazy deoptimization (e.g.
|
||||
// through compilation dependency finalization actions).
|
||||
// - Maglev (currently) disallows marking an opcode as both can_lazy_deopt
|
||||
// and can_eager_deopt.
|
||||
// - Maglev's JumpLoop opcode (the logical caller of this runtime function)
|
||||
// is marked as can_eager_deopt since OSR'ing to Turbofan involves
|
||||
// deoptimizing to Ignition under the hood.
|
||||
// - Thus this runtime function *must not* trigger a lazy deopt, and
|
||||
// therefore cannot trigger synchronous Turbofan compilation (see above).
|
||||
//
|
||||
// We solve this synchronous OSR case by bailing out early to Ignition, and
|
||||
// letting it handle OSR. How do we trigger the early bailout? Returning
|
||||
// any non-null Code from this function triggers the deopt in JumpLoop.
|
||||
return function->code();
|
||||
}
|
||||
|
||||
if (V8_LIKELY(isolate->concurrent_recompilation_enabled() &&
|
||||
v8_flags.concurrent_osr && v8_flags.turbofan)) {
|
||||
return CompileOptimizedOSR(isolate, function, osr_offset);
|
||||
} else {
|
||||
function->MarkForOptimization(isolate, CodeKind::TURBOFAN,
|
||||
ConcurrencyMode::kSynchronous);
|
||||
return function->code();
|
||||
}
|
||||
return CompileOptimizedOSR(isolate, function, osr_offset);
|
||||
}
|
||||
|
||||
RUNTIME_FUNCTION(Runtime_LogOrTraceOptimizedOSREntry) {
|
||||
|
21
test/mjsunit/maglev/regress/regress-1394279.js
Normal file
21
test/mjsunit/maglev/regress/regress-1394279.js
Normal file
@ -0,0 +1,21 @@
|
||||
// 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.
|
||||
//
|
||||
// Flags: --allow-natives-syntax --maglev --single-threaded
|
||||
|
||||
function foo(b) {
|
||||
foo.bind();
|
||||
if (b) {
|
||||
%OptimizeFunctionOnNextCall(foo);
|
||||
}
|
||||
for (let i = 0; i < 10000; i++) {}
|
||||
foo instanceof foo;
|
||||
}
|
||||
|
||||
%PrepareFunctionForOptimization(foo);
|
||||
foo(false);
|
||||
%OptimizeMaglevOnNextCall(foo);
|
||||
foo(true);
|
||||
foo.prototype = foo;
|
||||
foo(true);
|
@ -70,7 +70,9 @@ INCOMPATIBLE_FLAGS_PER_VARIANT = {
|
||||
],
|
||||
"nooptimization": ["--always-turbofan"],
|
||||
"slow_path": ["--no-force-slow-path"],
|
||||
"stress_concurrent_allocation": ["--single-threaded-gc", "--predictable"],
|
||||
"stress_concurrent_allocation": [
|
||||
"--single-threaded", "--single-threaded-gc", "--predictable"
|
||||
],
|
||||
"stress_concurrent_inlining": [
|
||||
"--single-threaded", "--predictable", "--lazy-feedback-allocation",
|
||||
"--assert-types", "--no-concurrent-recompilation"
|
||||
@ -127,7 +129,8 @@ INCOMPATIBLE_FLAGS_PER_BUILD_VARIABLE = {
|
||||
# implications defined in flag-definitions.h.
|
||||
INCOMPATIBLE_FLAGS_PER_EXTRA_FLAG = {
|
||||
"--concurrent-recompilation": [
|
||||
"--predictable", "--assert-types", "--turboshaft-assert-types"
|
||||
"--predictable", "--assert-types", "--turboshaft-assert-types",
|
||||
"--single-threaded"
|
||||
],
|
||||
"--parallel-compile-tasks-for-eager-toplevel": ["--predictable"],
|
||||
"--parallel-compile-tasks-for-lazy": ["--predictable"],
|
||||
|
Loading…
Reference in New Issue
Block a user