[wasm] Inlining improvements

Changes:
- Limit how often a function can be inlined, mostly to constrain
  recursive-function inlining.
- Move call count limiting earlier (to WasmInliner::ReduceCall), and
  guard it behind the flags that are required to generate call counts.

Bug: v8:12166
Change-Id: Ie3c140daff110e08fe7103ee79393ea27ae49bb2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3865918Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82880}

src/compiler/wasm-inlining.cc

@@ -73,19 +73,38 @@ Reduction WasmInliner::ReduceCall(Node* call) {
     Trace(call, inlinee_index, "imported function");
     return NoChange();
   }
-  if (inlinee_index == function_index_) {
-    Trace(call, inlinee_index, "recursive call");
+
+  // We limit the times a function can be inlined to avoid repeatedly inlining
+  // recursive calls. Since we only check here (and not in {Finalize}), it is
+  // possible to exceed this limit if we find a large number of calls in a
+  // single pass.
+  constexpr int kMaximumInlinedCallsPerFunction = 3;
+  if (function_inlining_count_[inlinee_index] >=
+      kMaximumInlinedCallsPerFunction) {
+    Trace(call, inlinee_index,
+          "too many inlined calls to (recursive?) function");
     return NoChange();
   }
 
-  Trace(call, inlinee_index, "adding to inlining candidates!");
-
-  int call_count = GetCallCount(call);
-
   CHECK_LT(inlinee_index, module()->functions.size());
   const wasm::WasmFunction* inlinee = &module()->functions[inlinee_index];
   base::Vector<const byte> function_bytes = wire_bytes_->GetCode(inlinee->code);
 
+  int call_count = GetCallCount(call);
+
+  int wire_byte_size = static_cast<int>(function_bytes.size());
+  int min_count_for_inlining = wire_byte_size / 2;
+
+  // If liftoff ran and collected call counts, only inline calls that have been
+  // invoked often, except for truly tiny functions.
+  if (v8_flags.liftoff && v8_flags.wasm_speculative_inlining &&
+      wire_byte_size >= 12 && call_count < min_count_for_inlining) {
+    Trace(call, inlinee_index, "not called often enough");
+    return NoChange();
+  }
+
+  Trace(call, inlinee_index, "adding to inlining candidates!");
+
   CandidateInfo candidate{call, inlinee_index, call_count,
                           function_bytes.length()};
 
@@ -123,14 +142,6 @@ void WasmInliner::Finalize() {
       Trace(candidate, "dead node");
       continue;
     }
-    int min_count_for_inlining = candidate.wire_byte_size / 2;
-    // Only inline calls that have been invoked often, except for truly tiny
-    // functions.
-    if (candidate.wire_byte_size >= 12 &&
-        candidate.call_count < min_count_for_inlining) {
-      Trace(candidate, "not called often enough");
-      continue;
-    }
     // We could build the candidate's graph first and consider its node count,
     // but it turns out that wire byte size and node count are quite strongly
     // correlated, at about 1.16 nodes per wire byte (measured for J2Wasm).
@@ -196,6 +207,8 @@ void WasmInliner::Finalize() {
     size_t additional_nodes = graph()->NodeCount() - subgraph_min_node_id;
     Trace(candidate, "inlining!");
     current_graph_size_ += additional_nodes;
+    DCHECK_GE(function_inlining_count_[candidate.inlinee_index], 0);
+    function_inlining_count_[candidate.inlinee_index]++;
 
     if (call->opcode() == IrOpcode::kCall) {
       InlineCall(call, inlinee_start, inlinee_end, lowered_sig,

src/compiler/wasm-inlining.h

@@ -111,6 +111,7 @@ class WasmInliner final : public AdvancedReducer {
                       LexicographicOrdering>
       inlining_candidates_;
   std::unordered_set<Node*> seen_;
+  std::unordered_map<uint32_t, int> function_inlining_count_;
 };
 
 }  // namespace compiler

test/mjsunit/wasm/inlining.js

@@ -420,3 +420,26 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
   let instance = builder.instantiate({});
   assertEquals(BigInt(21), instance.exports.main(BigInt(10), 11));
 })();
+
+(function InliningRecursiveTest() {
+  print(arguments.callee.name);
+
+  let builder = new WasmModuleBuilder();
+
+  let factorial = builder
+    .addFunction("factorial", kSig_i_i)
+    .addBody([kExprLocalGet, 0, kExprI32Const, 1, kExprI32LeS,
+              kExprIf, kWasmVoid, kExprI32Const, 1, kExprReturn, kExprEnd,
+              kExprLocalGet, 0, kExprI32Const, 1, kExprI32Sub,
+              kExprCallFunction, 0,
+              kExprLocalGet, 0, kExprI32Mul]);
+
+  builder.addFunction("main", kSig_i_i)
+    .addBody([kExprLocalGet, 0, kExprCallFunction, factorial.index])
+    .exportFunc();
+
+  let instance = builder.instantiate({});
+  assertEquals(1, instance.exports.main(1));
+  // {factorial} should not be fully inlined in the trace.
+  assertEquals(120, instance.exports.main(5));
+})();