[wasm] Only suspend on promise for stack-switching

Currently, the stack-switching import wrapper always suspends. Only
suspend if the returned value is a promise, otherwise just convert and
return the value back to wasm.

R=ahaas@chromium.org
CC=fgm@chromium.org

Bug: v8:12191
Change-Id: I26e7a3921aeae30fcce7f0ccc98d790a1a6f8c35
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3440655
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78980}
This commit is contained in:
Thibaud Michaud 2022-02-07 15:17:01 +01:00 committed by V8 LUCI CQ
parent d75f761334
commit 7b19d05b66
2 changed files with 49 additions and 9 deletions

View File

@ -41,6 +41,7 @@
#include "src/logging/counters.h"
#include "src/logging/log.h"
#include "src/objects/heap-number.h"
#include "src/objects/instance-type.h"
#include "src/roots/roots.h"
#include "src/tracing/trace-event.h"
#include "src/trap-handler/trap-handler.h"
@ -7031,16 +7032,29 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
global_proxy);
}
Node* BuildSuspend(Node* promise, Node* suspender) {
Node* BuildSuspend(Node* value, Node* api_function_ref,
MachineRepresentation rep) {
// If value is a promise, suspend to the js-to-wasm prompt, and resume later
// with the promise's resolved value.
auto resume = gasm_->MakeLabel(rep);
gasm_->GotoIf(IsSmi(value), &resume, value);
gasm_->GotoIfNot(gasm_->HasInstanceType(value, JS_PROMISE_TYPE), &resume,
BranchHint::kTrue, value);
Node* suspender = gasm_->Load(
MachineType::TaggedPointer(), api_function_ref,
wasm::ObjectAccess::ToTagged(WasmApiFunctionRef::kSuspenderOffset));
auto* call_descriptor = GetBuiltinCallDescriptor(
Builtin::kWasmSuspend, zone_, StubCallMode::kCallWasmRuntimeStub);
Node* call_target = mcgraph()->RelocatableIntPtrConstant(
wasm::WasmCode::kWasmSuspend, RelocInfo::WASM_STUB_CALL);
Node* args[] = {promise, suspender};
Node* args[] = {value, suspender};
Node* chained_promise =
BuildCallToRuntime(Runtime::kWasmCreateResumePromise, args, 2);
return gasm_->Call(call_descriptor, call_target, chained_promise,
suspender);
Node* resolved =
gasm_->Call(call_descriptor, call_target, chained_promise, suspender);
gasm_->Goto(&resume, resolved);
gasm_->Bind(&resume);
return resume.PhiAt(0);
}
// For wasm-to-js wrappers, parameter 0 is a WasmApiFunctionRef.
@ -7107,11 +7121,11 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
DCHECK_EQ(pos, args.size());
call = gasm_->Call(call_descriptor, pos, args.begin());
if (suspend == wasm::kSuspend) {
Node* suspender =
gasm_->Load(MachineType::TaggedPointer(), Param(0),
wasm::ObjectAccess::ToTagged(
WasmApiFunctionRef::kSuspenderOffset));
call = BuildSuspend(call, suspender);
MachineRepresentation rep =
sig_->return_count() >= 1
? sig_->GetReturn(0).machine_representation()
: MachineRepresentation::kNone;
call = BuildSuspend(call, Param(0), rep);
}
break;
}

View File

@ -167,3 +167,29 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
let wrapper = suspender.returnPromiseOnSuspend(instance.exports.test);
wrapper();
})();
// Check that the suspender does not suspend if the import's
// return value is not a promise.
(function TestStackSwitchNoPromise() {
print(arguments.callee.name);
let builder = new WasmModuleBuilder();
builder.addGlobal(kWasmI32, true).exportAs('g');
import_index = builder.addImport('m', 'import', kSig_i_v);
builder.addFunction("test", kSig_i_v)
.addBody([
kExprCallFunction, import_index, // suspend
kExprGlobalSet, 0, // resume
kExprGlobalGet, 0,
]).exportFunc();
let suspender = new WebAssembly.Suspender();
function js_import() {
return 42
};
let wasm_js_import = new WebAssembly.Function({parameters: [], results: ['externref']}, js_import);
let suspending_wasm_js_import = suspender.suspendOnReturnedPromise(wasm_js_import);
let instance = builder.instantiate({m: {import: suspending_wasm_js_import}});
let wrapped_export = suspender.returnPromiseOnSuspend(instance.exports.test);
let result = wrapped_export();
// TODO(thibaudm): Check the result's value once this is supported.
assertEquals(42, instance.exports.g.value);
})();