Skip to content

Commit

Permalink
[maglev] Hard limit on inlining depth
Browse files Browse the repository at this point in the history
Set a hard max inlining depth that also counts for small functions.

Bug: chromium:1487583
Change-Id: I7a55a5c506370713ad02a4668fb0f0c6b2cdb9a6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4907917
Commit-Queue: Olivier Flückiger <olivf@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Olivier Flückiger <olivf@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#90243}
  • Loading branch information
o- authored and V8 LUCI CQ committed Oct 4, 2023
1 parent 9399f75 commit 9c6afe3
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 4 deletions.
11 changes: 8 additions & 3 deletions src/flags/flag-definitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,12 +546,17 @@ DEFINE_UINT(
DEFINE_BOOL(concurrent_maglev_high_priority_threads, false,
"use high priority compiler threads for concurrent Maglev")

DEFINE_INT(max_maglev_inline_depth, 1,
"max depth of functions that Maglev will inline")
DEFINE_INT(
max_maglev_inline_depth, 1,
"max depth of functions that Maglev will inline excl. small functions")
DEFINE_INT(
max_maglev_hard_inline_depth, 10,
"max depth of functions that Maglev will inline incl. small functions")
DEFINE_INT(max_maglev_inlined_bytecode_size, 460,
"maximum size of bytecode for a single inlining")
DEFINE_INT(max_maglev_inlined_bytecode_size_cumulative, 920,
"maximum cumulative size of bytecode considered for inlining")
"maximum cumulative size of bytecode considered for inlining excl. "
"small functions")
DEFINE_INT(max_maglev_inlined_bytecode_size_small, 27,
"maximum size of bytecode considered for small function inlining")
DEFINE_FLOAT(min_maglev_inlining_frequency, 0.10,
Expand Down
15 changes: 14 additions & 1 deletion src/maglev/maglev-graph-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5314,6 +5314,17 @@ bool MaglevGraphBuilder::ShouldInlineCall(
TRACE_CANNOT_INLINE("it has not been compiled/run with feedback yet");
return false;
}
// TODO(olivf): This is a temporary stopgap to prevent infinite recursion when
// inlining, because we currently excempt small functions from some of the
// negative heuristics. We should refactor these heuristics and make sure they
// make sense in the presence of (mutually) recursive inlining. Please do
// *not* return true before this check.
if (inlining_depth() > v8_flags.max_maglev_hard_inline_depth) {
TRACE_CANNOT_INLINE("inlining depth ("
<< inlining_depth() << ") >= hard-max-depth ("
<< v8_flags.max_maglev_hard_inline_depth << ")");
return false;
}
if (compilation_unit_->shared_function_info().equals(shared)) {
TRACE_CANNOT_INLINE("direct recursion");
return false;
Expand Down Expand Up @@ -5355,7 +5366,9 @@ bool MaglevGraphBuilder::ShouldInlineCall(
return false;
}
if (bytecode.length() < v8_flags.max_maglev_inlined_bytecode_size_small) {
TRACE_INLINING(" inlining " << shared << ": small function");
TRACE_INLINING(" inlining "
<< shared
<< ": small function, skipping max-size and max-depth");
return true;
}
if (bytecode.length() > v8_flags.max_maglev_inlined_bytecode_size) {
Expand Down
22 changes: 22 additions & 0 deletions test/mjsunit/regress/regress-crbug-1487583.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2023 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

// Triggers mutual inlining.
function bar(x) {
foo([]);
}
function foo(arr) {
arr.forEach(bar);
}

foo([]);
%PrepareFunctionForOptimization(foo);
%PrepareFunctionForOptimization(bar);
foo([]);
bar([]);
foo([]);
%OptimizeFunctionOnNextCall(foo);
foo([]);

0 comments on commit 9c6afe3

Please sign in to comment.