Skip to content

V
V8
Commit 3ad23424 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot
Browse files
Options

Revert "[compiler] Emit a function-entry stack check on OSR-entry" 

This reverts commit 8703c38d.

Reason for revert: New test is timing out on gc-stress (e.g. https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/31726/overview)

Original change's description:
> [compiler] Emit a function-entry stack check on OSR-entry
>
> This CL extends the smarter function-entry stack check logic (see
> v8:9534) to OSR'd code. These smarter stack checks prevent
> overflowing the stack during deoptimization.
>
> The challenge for both function-entry (FE) and OSR-entry (OE) stack
> checks is that there is no dedicated physical StackCheck to
> deoptimize into. For more context: the physical StackCheck bytecode
> was removed in crrev.com/c/1914218.
>
> FE stack checks solve this by using a marker bailout id to signify
> a deopt bytecode offset before the first bytecode.
>
> In this CL, OE stack checks take a similar approach by using the
> OSR'd loop's JumpLoop bytecode, which is conceptually immediately
> before the OSR'd loop header.
>
> When a stack overflow at an OE stack check occurs: %StackGuard
> may cause a lazy deopt on return to the optimized OSR code,
> causing re-execution of the JumpLoop handler in the
> InterpreterEnterBytecodeAdvance builtin, ultimately continuing
> execution the interpreter at the first bytecode of the OSR'd loop
> header.
>
> Bug: chromium:1034322, v8:9534
> Change-Id: I1ae88a08702cde9a5eb84a451a9f1acc41204d5c
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2625872
> Auto-Submit: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#72153}

TBR=neis@chromium.org,jgruber@chromium.org,solanes@chromium.org

Change-Id: Ie72f2e2927ffa83d595aad0d88c606d422f953a2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1034322
Bug: v8:9534
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2637858Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72158}
parent 8e93a32d
Hide whitespace changes
Inline Side-by-side
Showing with 27 additions and 101 deletions
src/compiler/bytecode-graph-builder.cc
View file @ 3ad23424
......@@ -207,54 +207,10 @@ class BytecodeGraphBuilder {
// Prepare information for lazy deoptimization. This information is attached
// to the given node and the output value produced by the node is combined.
//
// The low-level chokepoint - use the variants below instead.
// Conceptually this frame state is "after" a given operation.
void PrepareFrameState(Node* node, OutputFrameStateCombine combine);
void PrepareFrameState(Node* node, OutputFrameStateCombine combine,
BailoutId bailout_id,
const BytecodeLivenessState* liveness);
// In the common case, frame states are conceptually "after" a given
// operation and at the current bytecode offset.
void PrepareFrameState(Node* node, OutputFrameStateCombine combine) {
if (!OperatorProperties::HasFrameStateInput(node->op())) return;
const int offset = bytecode_iterator().current_offset();
return PrepareFrameState(node, combine, BailoutId(offset),
bytecode_analysis().GetOutLivenessFor(offset));
}
// For function-entry stack checks, they're conceptually "before" the first
// bytecode and at a special marker bytecode offset.
// In the case of FE stack checks, the current bytecode is also the first
// bytecode, so we use a special marker bytecode offset to signify a virtual
// bytecode before the first physical bytecode.
void PrepareFrameStateForFunctionEntryStackCheck(Node* node) {
DCHECK_EQ(bytecode_iterator().current_offset(), 0);
DCHECK(OperatorProperties::HasFrameStateInput(node->op()));
DCHECK(node->opcode() == IrOpcode::kJSStackCheck);
return PrepareFrameState(node, OutputFrameStateCombine::Ignore(),
BailoutId(kFunctionEntryBytecodeOffset),
bytecode_analysis().GetInLivenessFor(0));
}
// For OSR-entry stack checks, they're conceptually "before" the first
// bytecode of the current loop. We implement this in a similar manner to
// function-entry (FE) stack checks above, i.e. we deopt at the predecessor
// of the current bytecode.
// In the case of OSR-entry stack checks, a physical predecessor bytecode
// exists: the JumpLoop bytecode. We attach to JumpLoop by using
// `bytecode_analysis().osr_bailout_id()` instead of current_offset (the
// former points at JumpLoop, the latter at the loop header, i.e. the target
// of JumpLoop).
void PrepareFrameStateForOSREntryStackCheck(Node* node) {
DCHECK_EQ(bytecode_iterator().current_offset(),
bytecode_analysis().osr_entry_point());
DCHECK(OperatorProperties::HasFrameStateInput(node->op()));
DCHECK(node->opcode() == IrOpcode::kJSStackCheck);
const int offset = bytecode_analysis().osr_bailout_id().ToInt();
return PrepareFrameState(node, OutputFrameStateCombine::Ignore(),
BailoutId(offset),
bytecode_analysis().GetOutLivenessFor(offset));
}
BailoutId bailout_id);
void BuildCreateArguments(CreateArgumentsType type);
Node* BuildLoadGlobal(NameRef name, uint32_t feedback_slot_index,
......@@ -348,7 +304,6 @@ class BytecodeGraphBuilder {
// StackChecks.
void BuildFunctionEntryStackCheck();
void BuildIterationBodyStackCheck();
void MaybeBuildOSREntryStackCheck();
// Control flow plumbing.
void BuildJump();
......@@ -479,7 +434,6 @@ class BytecodeGraphBuilder {
Environment* environment_;
bool const osr_;
int currently_peeled_loop_offset_;
bool is_osr_entry_stack_check_pending_;
const bool skip_first_stack_check_;
......@@ -1068,7 +1022,6 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
environment_(nullptr),
osr_(!osr_offset.IsNone()),
currently_peeled_loop_offset_(-1),
is_osr_entry_stack_check_pending_(osr_),
skip_first_stack_check_(flags &
BytecodeGraphBuilderFlag::kSkipFirstStackCheck),
merge_environments_(local_zone),
......@@ -1286,18 +1239,36 @@ void BytecodeGraphBuilder::PrepareEagerCheckpoint() {
#endif // DEBUG
}
void BytecodeGraphBuilder::PrepareFrameState(
Node* node, OutputFrameStateCombine combine, BailoutId bailout_id,
const BytecodeLivenessState* liveness) {
void BytecodeGraphBuilder::PrepareFrameState(Node* node,
OutputFrameStateCombine combine) {
if (OperatorProperties::HasFrameStateInput(node->op())) {
PrepareFrameState(node, combine,
BailoutId(bytecode_iterator().current_offset()));
}
}
void BytecodeGraphBuilder::PrepareFrameState(Node* node,
OutputFrameStateCombine combine,
BailoutId bailout_id) {
if (OperatorProperties::HasFrameStateInput(node->op())) {
// Add the frame state for after the operation. The node in question has
// already been created and had a {Dead} frame state input up until now.
DCHECK_EQ(1, OperatorProperties::GetFrameStateInputCount(node->op()));
DCHECK_EQ(IrOpcode::kDead,
NodeProperties::GetFrameStateInput(node)->opcode());
DCHECK_IMPLIES(bailout_id.ToInt() == kFunctionEntryBytecodeOffset,
bytecode_iterator().current_offset() == 0);
// If we have kFunctionEntryBytecodeOffset as the bailout_id, we want to get
// the liveness at the moment of function entry. This is the same as the IN
// liveness of the first actual bytecode.
const BytecodeLivenessState* liveness_after =
bailout_id.ToInt() == kFunctionEntryBytecodeOffset
? bytecode_analysis().GetInLivenessFor(0)
: bytecode_analysis().GetOutLivenessFor(bailout_id.ToInt());
Node* frame_state_after =
environment()->Checkpoint(bailout_id, combine, liveness);
environment()->Checkpoint(bailout_id, combine, liveness_after);
NodeProperties::ReplaceFrameStateInput(node, frame_state_after);
}
}
......@@ -1407,7 +1378,8 @@ void BytecodeGraphBuilder::BuildFunctionEntryStackCheck() {
if (!skip_first_stack_check()) {
Node* node =
NewNode(javascript()->StackCheck(StackCheckKind::kJSFunctionEntry));
PrepareFrameStateForFunctionEntryStackCheck(node);
PrepareFrameState(node, OutputFrameStateCombine::Ignore(),
BailoutId(kFunctionEntryBytecodeOffset));
}
}
......@@ -1417,15 +1389,6 @@ void BytecodeGraphBuilder::BuildIterationBodyStackCheck() {
environment()->RecordAfterState(node, Environment::kAttachFrameState);
}
void BytecodeGraphBuilder::MaybeBuildOSREntryStackCheck() {
if (V8_UNLIKELY(is_osr_entry_stack_check_pending_)) {
is_osr_entry_stack_check_pending_ = false;
Node* node =
NewNode(javascript()->StackCheck(StackCheckKind::kJSFunctionEntry));
PrepareFrameStateForOSREntryStackCheck(node);
}
}
// We will iterate through the OSR loop, then its parent, and so on
// until we have reached the outmost loop containing the OSR loop. We do
// not generate nodes for anything before the outermost loop.
......@@ -1506,13 +1469,6 @@ void BytecodeGraphBuilder::VisitSingleBytecode() {
if (environment() != nullptr) {
BuildLoopHeaderEnvironment(current_offset);
// The OSR-entry stack check must be emitted during the first call to
// VisitSingleBytecode in an OSR'd function. We don't know if that call
// will be made from AdvanceToOsrEntryAndPeelLoops or from VisitBytecodes,
// therefore we insert the logic here inside VisitSingleBytecode itself.
MaybeBuildOSREntryStackCheck();
switch (bytecode_iterator().current_bytecode()) {
#define BYTECODE_CASE(name, ...) \
case interpreter::Bytecode::k##name: \
......
test/mjsunit/regress/regress-1034322.js deleted 100644 → 0
View file @ 8e93a32d
// Copyright 2021 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 --stack-size=103
let ticks = 0;
function v0() {
try { v1(); } catch {}
// This triggers the deopt that may overflow the stack.
try { undefined[null] = null; } catch {}
}
function v1() {
while (!v0()) {
// Trigger OSR early to get a crashing case asap.
if (ticks == 5) %OptimizeOsr();
// With the bug fixed, there's no easy way to trigger termination. Instead,
// run until we reach a certain number of ticks. The crash triggers locally
// at tick 7562, thus running until 20k ticks to be somewhat safe.
if (ticks >= 20000) exit(0);
ticks++;
}
}
%PrepareFunctionForOptimization(v0);
%PrepareFunctionForOptimization(v1);
v0();
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