Commit c0956fbd authored by Manos Koukoutos's avatar Manos Koukoutos Committed by V8 LUCI CQ

[wasm] Fix bugs in inlining on 32-bit platforms

- Use the lowered 32-bit signature when linking the inlined and caller
  graphs.
- Tolerate non-projection uses of Call nodes when linking the graphs.
  These can be left over by Int64Lowering.
- Drive-by: Inline really small functions even if their call count is
  low.

Bug: v8:12166
Change-Id: I5b472d3f617f2f23820a5d142102c0a6c5c769dc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3720715Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81386}
parent 8e49ce29
......@@ -8631,6 +8631,45 @@ CallDescriptor* GetI32WasmCallDescriptor(
zone, call_descriptor, 2, MachineType::Int64(), MachineType::Int32());
}
namespace {
const wasm::FunctionSig* ReplaceTypeInSig(Zone* zone,
const wasm::FunctionSig* sig,
wasm::ValueType from,
wasm::ValueType to,
size_t num_replacements) {
size_t param_occurences =
std::count(sig->parameters().begin(), sig->parameters().end(), from);
size_t return_occurences =
std::count(sig->returns().begin(), sig->returns().end(), from);
if (param_occurences == 0 && return_occurences == 0) return sig;
wasm::FunctionSig::Builder builder(
zone, sig->return_count() + return_occurences * (num_replacements - 1),
sig->parameter_count() + param_occurences * (num_replacements - 1));
for (wasm::ValueType ret : sig->returns()) {
if (ret == from) {
for (size_t i = 0; i < num_replacements; i++) builder.AddReturn(to);
} else {
builder.AddReturn(ret);
}
}
for (wasm::ValueType param : sig->parameters()) {
if (param == from) {
for (size_t i = 0; i < num_replacements; i++) builder.AddParam(to);
} else {
builder.AddParam(param);
}
}
return builder.Build();
}
} // namespace
const wasm::FunctionSig* GetI32Sig(Zone* zone, const wasm::FunctionSig* sig) {
return ReplaceTypeInSig(zone, sig, wasm::kWasmI64, wasm::kWasmI32, 2);
}
AssemblerOptions WasmAssemblerOptions() {
AssemblerOptions options;
// Relocation info required to serialize {WasmCode} for proper functions.
......
......@@ -865,6 +865,9 @@ V8_EXPORT_PRIVATE CallDescriptor* GetWasmCallDescriptor(
V8_EXPORT_PRIVATE CallDescriptor* GetI32WasmCallDescriptor(
Zone* zone, const CallDescriptor* call_descriptor);
V8_EXPORT_PRIVATE const wasm::FunctionSig* GetI32Sig(
Zone* zone, const wasm::FunctionSig* sig);
AssemblerOptions WasmAssemblerOptions();
AssemblerOptions WasmStubAssemblerOptions();
......
......@@ -124,7 +124,10 @@ void WasmInliner::Finalize() {
continue;
}
int min_count_for_inlining = candidate.wire_byte_size / 2;
if (candidate.call_count < min_count_for_inlining) {
// 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;
}
......@@ -138,16 +141,20 @@ void WasmInliner::Finalize() {
const wasm::WasmFunction* inlinee =
&module()->functions[candidate.inlinee_index];
DCHECK_EQ(inlinee->sig->parameter_count(),
const wasm::FunctionSig* lowered_sig =
mcgraph_->machine()->Is64() ? inlinee->sig
: GetI32Sig(zone(), inlinee->sig);
DCHECK_EQ(lowered_sig->parameter_count(),
call->op()->ValueInputCount() - 2);
#if DEBUG
// The two first parameters in the call are the function and instance, and
// then come the wasm function parameters.
for (uint32_t i = 0; i < inlinee->sig->parameter_count(); i++) {
for (uint32_t i = 0; i < lowered_sig->parameter_count(); i++) {
if (!NodeProperties::IsTyped(call->InputAt(i + 2))) continue;
wasm::TypeInModule param_type =
NodeProperties::GetType(call->InputAt(i + 2)).AsWasm();
CHECK(IsSubtypeOf(param_type.type, inlinee->sig->GetParam(i),
CHECK(IsSubtypeOf(param_type.type, lowered_sig->GetParam(i),
param_type.module, module()));
}
#endif
......@@ -191,7 +198,7 @@ void WasmInliner::Finalize() {
current_graph_size_ += additional_nodes;
if (call->opcode() == IrOpcode::kCall) {
InlineCall(call, inlinee_start, inlinee_end, inlinee->sig,
InlineCall(call, inlinee_start, inlinee_end, lowered_sig,
subgraph_min_node_id);
} else {
InlineTailCall(call, inlinee_start, inlinee_end);
......@@ -434,6 +441,12 @@ void WasmInliner::InlineCall(Node* call, Node* callee_start, Node* callee_end,
Int32Matcher(NodeProperties::GetValueInput(return_nodes[0], 0)).Is(0));
int const return_arity = return_nodes[0]->op()->ValueInputCount() - 1;
NodeVector values(zone());
#if DEBUG
for (Node* const return_node : return_nodes) {
// 3 = effect, control, first 0 return value.
CHECK_EQ(return_arity, return_node->InputCount() - 3);
}
#endif
for (int i = 0; i < return_arity; i++) {
NodeVector ith_values(zone());
for (Node* const return_node : return_nodes) {
......@@ -464,8 +477,12 @@ void WasmInliner::InlineCall(Node* call, Node* callee_start, Node* callee_end,
for (Edge use_edge : call->use_edges()) {
if (NodeProperties::IsValueEdge(use_edge)) {
Node* use = use_edge.from();
DCHECK_EQ(use->opcode(), IrOpcode::kProjection);
// Other nodes are unreachable leftovers from Int32Lowering.
if (use->opcode() == IrOpcode::kProjection) {
ReplaceWithValue(use, values[ProjectionIndexOf(use->op())]);
} else {
DCHECK(mcgraph()->machine()->Is32());
}
}
}
// All value inputs are replaced by the above loop, so it is ok to use
......
// 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: --no-liftoff --wasm-inlining
// This tests that inlining tolerates multi-return call uses that are not
// projections after Int64Lowering.
d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
let builder = new WasmModuleBuilder();
let callee1 = builder.addFunction("callee1", kSig_l_l)
.addBody([kExprLocalGet, 0, kExprI64Const, 1, kExprI64Add]);
let callee2 = builder.addFunction("callee2", kSig_l_l)
.addBody([kExprLocalGet, 0, kExprI64Const, 1, kExprI64Sub]);
builder.addFunction("caller", kSig_l_l)
.addBody([kExprLocalGet, 0,
kExprI64Const, 0,
kExprI64GtS,
kExprIf, kWasmI64,
kExprLocalGet, 0, kExprCallFunction, 0,
kExprElse,
kExprLocalGet, 0, kExprCallFunction, 1,
kExprEnd])
.exportFunc();
let instance = builder.instantiate();
assertEquals(5n, instance.exports.caller(4n));
assertEquals(-9n, instance.exports.caller(-8n));
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