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

[wasm][turbofan] Remove stack checks/tracing from inlinee, add tests

We add an option to BuildTFGraph to not emit stack checks and call
tracing and use it in inlined functions.
Also, we add tests for zero/multiple return values, as well as infinite
loops in the inlined function.

Bug: v8:12166
Change-Id: I5f34c57d9870592085804853ff23ba94897cc8d5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3141589Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76718}
parent 398f0314
...@@ -7816,9 +7816,10 @@ bool BuildGraphForWasmFunction(wasm::CompilationEnv* env, ...@@ -7816,9 +7816,10 @@ bool BuildGraphForWasmFunction(wasm::CompilationEnv* env,
WasmGraphBuilder builder(env, mcgraph->zone(), mcgraph, func_body.sig, WasmGraphBuilder builder(env, mcgraph->zone(), mcgraph, func_body.sig,
source_positions); source_positions);
auto* allocator = wasm::GetWasmEngine()->allocator(); auto* allocator = wasm::GetWasmEngine()->allocator();
wasm::VoidResult graph_construction_result = wasm::BuildTFGraph( wasm::VoidResult graph_construction_result =
allocator, env->enabled_features, env->module, &builder, detected, wasm::BuildTFGraph(allocator, env->enabled_features, env->module,
func_body, loop_infos, node_origins, func_index); &builder, detected, func_body, loop_infos,
node_origins, func_index, wasm::kInstrumentEndpoints);
if (graph_construction_result.failed()) { if (graph_construction_result.failed()) {
if (FLAG_trace_wasm_compiler) { if (FLAG_trace_wasm_compiler) {
StdoutStream{} << "Compilation failed: " StdoutStream{} << "Compilation failed: "
......
...@@ -51,7 +51,8 @@ Reduction WasmInliner::ReduceCall(Node* call) { ...@@ -51,7 +51,8 @@ Reduction WasmInliner::ReduceCall(Node* call) {
Graph::SubgraphScope scope(graph()); Graph::SubgraphScope scope(graph());
result = wasm::BuildTFGraph(zone()->allocator(), env_->enabled_features, result = wasm::BuildTFGraph(zone()->allocator(), env_->enabled_features,
module(), &builder, &detected, inlinee_body, module(), &builder, &detected, inlinee_body,
&infos, node_origins_, inlinee_index_); &infos, node_origins_, inlinee_index_,
wasm::kDoNotInstrumentEndpoints);
inlinee_start = graph()->start(); inlinee_start = graph()->start();
inlinee_end = graph()->end(); inlinee_end = graph()->end();
} }
...@@ -60,9 +61,7 @@ Reduction WasmInliner::ReduceCall(Node* call) { ...@@ -60,9 +61,7 @@ Reduction WasmInliner::ReduceCall(Node* call) {
return InlineCall(call, inlinee_start, inlinee_end); return InlineCall(call, inlinee_start, inlinee_end);
} }
// TODO(12166): Handle exceptions. // TODO(12166): Handle exceptions and tail calls.
// TODO(12166): Test multiple/zero returns, infinite loops.
// TODO(12166): Remove stack checks and wasm tracing from inlinee.
Reduction WasmInliner::InlineCall(Node* call, Node* callee_start, Reduction WasmInliner::InlineCall(Node* call, Node* callee_start,
Node* callee_end) { Node* callee_end) {
DCHECK_EQ(call->opcode(), IrOpcode::kCall); DCHECK_EQ(call->opcode(), IrOpcode::kCall);
......
...@@ -109,9 +109,12 @@ class WasmGraphBuildingInterface { ...@@ -109,9 +109,12 @@ class WasmGraphBuildingInterface {
: ControlBase(std::forward<Args>(args)...) {} : ControlBase(std::forward<Args>(args)...) {}
}; };
explicit WasmGraphBuildingInterface(compiler::WasmGraphBuilder* builder, WasmGraphBuildingInterface(compiler::WasmGraphBuilder* builder,
int func_index) int func_index,
: builder_(builder), func_index_(func_index) {} EndpointInstrumentationMode instrumentation)
: builder_(builder),
func_index_(func_index),
instrumentation_(instrumentation) {}
void StartFunction(FullDecoder* decoder) { void StartFunction(FullDecoder* decoder) {
// Get the branch hints map for this function (if available) // Get the branch hints map for this function (if available)
...@@ -155,7 +158,9 @@ class WasmGraphBuildingInterface { ...@@ -155,7 +158,9 @@ class WasmGraphBuildingInterface {
} }
LoadContextIntoSsa(ssa_env); LoadContextIntoSsa(ssa_env);
if (FLAG_trace_wasm) builder_->TraceFunctionEntry(decoder->position()); if (FLAG_trace_wasm && instrumentation_ == kInstrumentEndpoints) {
builder_->TraceFunctionEntry(decoder->position());
}
} }
// Reload the instance cache entries into the Ssa Environment. // Reload the instance cache entries into the Ssa Environment.
...@@ -165,7 +170,11 @@ class WasmGraphBuildingInterface { ...@@ -165,7 +170,11 @@ class WasmGraphBuildingInterface {
void StartFunctionBody(FullDecoder* decoder, Control* block) {} void StartFunctionBody(FullDecoder* decoder, Control* block) {}
void FinishFunction(FullDecoder*) { builder_->PatchInStackCheckIfNeeded(); } void FinishFunction(FullDecoder*) {
if (instrumentation_ == kInstrumentEndpoints) {
builder_->PatchInStackCheckIfNeeded();
}
}
void OnFirstError(FullDecoder*) {} void OnFirstError(FullDecoder*) {}
...@@ -477,7 +486,7 @@ class WasmGraphBuildingInterface { ...@@ -477,7 +486,7 @@ class WasmGraphBuildingInterface {
: decoder->stack_value(ret_count + drop_values); : decoder->stack_value(ret_count + drop_values);
GetNodes(values.begin(), stack_base, ret_count); GetNodes(values.begin(), stack_base, ret_count);
} }
if (FLAG_trace_wasm) { if (FLAG_trace_wasm && instrumentation_ == kInstrumentEndpoints) {
builder_->TraceFunctionExit(base::VectorOf(values), decoder->position()); builder_->TraceFunctionExit(base::VectorOf(values), decoder->position());
} }
builder_->Return(base::VectorOf(values)); builder_->Return(base::VectorOf(values));
...@@ -1159,6 +1168,7 @@ class WasmGraphBuildingInterface { ...@@ -1159,6 +1168,7 @@ class WasmGraphBuildingInterface {
const BranchHintMap* branch_hints_ = nullptr; const BranchHintMap* branch_hints_ = nullptr;
// Tracks loop data for loop unrolling. // Tracks loop data for loop unrolling.
std::vector<compiler::WasmLoopInfo> loop_infos_; std::vector<compiler::WasmLoopInfo> loop_infos_;
EndpointInstrumentationMode instrumentation_;
TFNode* effect() { return builder_->effect(); } TFNode* effect() { return builder_->effect(); }
...@@ -1594,10 +1604,12 @@ DecodeResult BuildTFGraph(AccountingAllocator* allocator, ...@@ -1594,10 +1604,12 @@ DecodeResult BuildTFGraph(AccountingAllocator* allocator,
WasmFeatures* detected, const FunctionBody& body, WasmFeatures* detected, const FunctionBody& body,
std::vector<compiler::WasmLoopInfo>* loop_infos, std::vector<compiler::WasmLoopInfo>* loop_infos,
compiler::NodeOriginTable* node_origins, compiler::NodeOriginTable* node_origins,
int func_index) { int func_index,
EndpointInstrumentationMode instrumentation) {
Zone zone(allocator, ZONE_NAME); Zone zone(allocator, ZONE_NAME);
WasmFullDecoder<Decoder::kFullValidation, WasmGraphBuildingInterface> decoder( WasmFullDecoder<Decoder::kFullValidation, WasmGraphBuildingInterface> decoder(
&zone, module, enabled, detected, body, builder, func_index); &zone, module, enabled, detected, body, builder, func_index,
instrumentation);
if (node_origins) { if (node_origins) {
builder->AddBytecodePositionDecorator(node_origins, &decoder); builder->AddBytecodePositionDecorator(node_origins, &decoder);
} }
......
...@@ -27,12 +27,18 @@ struct FunctionBody; ...@@ -27,12 +27,18 @@ struct FunctionBody;
class WasmFeatures; class WasmFeatures;
struct WasmModule; struct WasmModule;
enum EndpointInstrumentationMode {
kDoNotInstrumentEndpoints,
kInstrumentEndpoints
};
V8_EXPORT_PRIVATE DecodeResult V8_EXPORT_PRIVATE DecodeResult
BuildTFGraph(AccountingAllocator* allocator, const WasmFeatures& enabled, BuildTFGraph(AccountingAllocator* allocator, const WasmFeatures& enabled,
const WasmModule* module, compiler::WasmGraphBuilder* builder, const WasmModule* module, compiler::WasmGraphBuilder* builder,
WasmFeatures* detected, const FunctionBody& body, WasmFeatures* detected, const FunctionBody& body,
std::vector<compiler::WasmLoopInfo>* loop_infos, std::vector<compiler::WasmLoopInfo>* loop_infos,
compiler::NodeOriginTable* node_origins, int func_index); compiler::NodeOriginTable* node_origins, int func_index,
EndpointInstrumentationMode instrumentation);
} // namespace wasm } // namespace wasm
} // namespace internal } // namespace internal
......
...@@ -384,15 +384,16 @@ void TestBuildingGraphWithBuilder(compiler::WasmGraphBuilder* builder, ...@@ -384,15 +384,16 @@ void TestBuildingGraphWithBuilder(compiler::WasmGraphBuilder* builder,
std::vector<compiler::WasmLoopInfo> loops; std::vector<compiler::WasmLoopInfo> loops;
DecodeResult result = DecodeResult result =
BuildTFGraph(zone->allocator(), WasmFeatures::All(), nullptr, builder, BuildTFGraph(zone->allocator(), WasmFeatures::All(), nullptr, builder,
&unused_detected_features, body, &loops, nullptr, 0); &unused_detected_features, body, &loops, nullptr, 0,
kInstrumentEndpoints);
if (result.failed()) { if (result.failed()) {
#ifdef DEBUG #ifdef DEBUG
if (!FLAG_trace_wasm_decoder) { if (!FLAG_trace_wasm_decoder) {
// Retry the compilation with the tracing flag on, to help in debugging. // Retry the compilation with the tracing flag on, to help in debugging.
FLAG_trace_wasm_decoder = true; FLAG_trace_wasm_decoder = true;
result = result = BuildTFGraph(zone->allocator(), WasmFeatures::All(), nullptr,
BuildTFGraph(zone->allocator(), WasmFeatures::All(), nullptr, builder, builder, &unused_detected_features, body, &loops,
&unused_detected_features, body, &loops, nullptr, 0); nullptr, 0, kInstrumentEndpoints);
} }
#endif #endif
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// Flags: --wasm-inlining // Flags: --wasm-inlining --no-liftoff
d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js"); d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
...@@ -24,3 +24,54 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js"); ...@@ -24,3 +24,54 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
let instance = builder.instantiate(); let instance = builder.instantiate();
assertEquals(instance.exports.main(10), 14); assertEquals(instance.exports.main(10), 14);
})(); })();
(function MultiReturnTest() {
let builder = new WasmModuleBuilder();
// f(x) = (x - 1, x + 1)
let callee = builder.addFunction("callee", kSig_ii_i)
.addBody([kExprLocalGet, 0, kExprI32Const, 1, kExprI32Sub,
kExprLocalGet, 0, kExprI32Const, 1, kExprI32Add]);
// g(x) = { let (a, b) = f(x); a * b}
builder.addFunction("main", kSig_i_i)
.addBody([kExprLocalGet, 0, kExprCallFunction, callee.index, kExprI32Mul])
.exportAs("main");
let instance = builder.instantiate();
assertEquals(instance.exports.main(10), 9 * 11);
})();
(function NoReturnTest() {
let builder = new WasmModuleBuilder();
let global = builder.addGlobal(kWasmI32, true);
let callee = builder.addFunction("callee", kSig_v_i)
.addBody([kExprLocalGet, 0, kExprGlobalSet, global.index]);
builder.addFunction("main", kSig_i_i)
.addBody([kExprLocalGet, 0, kExprCallFunction, callee.index,
kExprGlobalGet, global.index])
.exportAs("main");
let instance = builder.instantiate();
assertEquals(instance.exports.main(10), 10);
})();
(function InfiniteLoopTest() {
let builder = new WasmModuleBuilder();
let callee = builder.addFunction("callee", kSig_i_i)
.addBody([kExprLoop, kWasmVoid,
kExprLocalGet, 0, kExprI32Const, 1, kExprI32Add,
kExprLocalSet, 0, kExprBr, 0,
kExprEnd,
kExprLocalGet, 0]);
builder.addFunction("main", kSig_i_i)
.addBody([kExprI32Const, 5, kExprCallFunction, callee.index,
kExprLocalGet, 0, kExprI32Add])
.exportAs("main");
builder.instantiate();
})();
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