Revert "[maglev] Prevent lazy deopts during maglev's JumpLoop (=OSR)"

This reverts commit 5d95bd39ca.

Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20-%20gc%20stress/2101/overview

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 commit 21969e8e24.
> This reverts commit 6bcbcfed5c.
>
> 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: Ib82d06ab8281f0e59a2af2b631bf93b25064df1f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4110745
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84925}
This commit is contained in:
Manos Koukoutos 2022-12-19 09:42:21 +00:00 committed by V8 LUCI CQ
parent 0b36f43e9e
commit 06a53cc0fa
4 changed files with 13 additions and 46 deletions

View File

@ -3048,10 +3048,9 @@ 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, 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).
// 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.
//
// See also: InterpreterAssembler::OnStackReplacement.

View File

@ -548,27 +548,18 @@ RUNTIME_FUNCTION(Runtime_CompileOptimizedOSRFromMaglev) {
DCHECK_EQ(frame->LookupCodeT().kind(), CodeKind::MAGLEV);
Handle<JSFunction> function = handle(frame->function(), isolate);
// 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.
if (V8_UNLIKELY(function->ActiveTierIsTurbofan())) {
return function->code();
}
return CompileOptimizedOSR(isolate, function, osr_offset);
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();
}
}
RUNTIME_FUNCTION(Runtime_LogOrTraceOptimizedOSREntry) {

View File

@ -1,21 +0,0 @@
// 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);

View File

@ -70,9 +70,7 @@ INCOMPATIBLE_FLAGS_PER_VARIANT = {
],
"nooptimization": ["--always-turbofan"],
"slow_path": ["--no-force-slow-path"],
"stress_concurrent_allocation": [
"--single-threaded", "--single-threaded-gc", "--predictable"
],
"stress_concurrent_allocation": ["--single-threaded-gc", "--predictable"],
"stress_concurrent_inlining": [
"--single-threaded", "--predictable", "--lazy-feedback-allocation",
"--assert-types", "--no-concurrent-recompilation"