Skip to content

Commit

Permalink
Merged: [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.

(cherry picked from commit 9c6afe3)

Change-Id: I46ff324f84f959c62052d83686832e7ded68a860
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4912944
Auto-Submit: Olivier Flückiger <[email protected]>
Reviewed-by: Toon Verwaest <[email protected]>
Commit-Queue: Toon Verwaest <[email protected]>
Cr-Commit-Position: refs/branch-heads/11.9@{#2}
Cr-Branched-From: 3eb7d73-refs/heads/11.9.169@{#1}
Cr-Branched-From: 1a13bf9-refs/heads/main@{#90218}
  • Loading branch information
o- authored and V8 LUCI CQ committed Oct 5, 2023
1 parent c9ef69a commit e08d42e
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 e08d42e

Please sign in to comment.