From b4487541965dd1b8fb295d5c188173d721496f76 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 18 Mar 2020 16:36:45 +0000 Subject: [PATCH] Bug 1620197 - Enable multiple results from WebAssembly functions r=lth This patch enables multi-value calls and returns, adding some tests, and conditionally disabling a couple expect-fail reftests that now pass. It also changes to make calling multi-result WebAssembly functions from JavaScript raise a run-time error, and likewise for calling out to multi-result JS function from wasm. Previously these would abort at stub generation time. Differential Revision: https://phabricator.services.mozilla.com/D65502 --HG-- extra : moz-landing-system : lando --- .../tests/wasm/multi-value/call-js.js | 23 ++++ .../tests/wasm/multi-value/call-ref.js | 31 +++++ .../tests/wasm/multi-value/call-run.js | 120 ++++++++++++++++++ .../tests/wasm/multi-value/call-validate.js | 47 +++++++ js/src/jit/BacktrackingAllocator.cpp | 2 +- js/src/jit/LIR.h | 3 +- js/src/jit/Lowering.cpp | 1 + js/src/jit/shared/LIR-shared.h | 28 +++- js/src/js.msg | 2 + js/src/wasm/WasmBaselineCompile.cpp | 2 +- js/src/wasm/WasmCode.h | 1 + js/src/wasm/WasmConstants.h | 5 - js/src/wasm/WasmInstance.cpp | 12 ++ js/src/wasm/WasmOpIter.h | 12 -- js/src/wasm/WasmStubs.cpp | 51 ++++++-- js/src/wasm/WasmTextToBinary.cpp | 23 +--- js/src/wasm/WasmTypes.h | 10 ++ js/src/wasm/WasmValidate.cpp | 8 -- .../jsapi/constructor/multi-value.any.js.ini | 3 +- 19 files changed, 319 insertions(+), 65 deletions(-) create mode 100644 js/src/jit-test/tests/wasm/multi-value/call-js.js create mode 100644 js/src/jit-test/tests/wasm/multi-value/call-ref.js create mode 100644 js/src/jit-test/tests/wasm/multi-value/call-run.js create mode 100644 js/src/jit-test/tests/wasm/multi-value/call-validate.js diff --git a/js/src/jit-test/tests/wasm/multi-value/call-js.js b/js/src/jit-test/tests/wasm/multi-value/call-js.js new file mode 100644 index 000000000000..27a505ec5af6 --- /dev/null +++ b/js/src/jit-test/tests/wasm/multi-value/call-js.js @@ -0,0 +1,23 @@ + +function expectRunFailure(text, pattern, imports) { + let instance = wasmEvalText(text, imports); + assertErrorMessage(() => instance.exports.run(), + TypeError, + pattern); +} + +expectRunFailure(` + (module + (import "env" "f" (func $f (result i32 i32))) + (func (export "run") (result i32) + (call $f) + i32.sub))`, + /calling JavaScript functions with multiple results from WebAssembly not yet implemented/, + { env: { f: () => [52, 10] } }); + +expectRunFailure(` + (module + (func (export "run") (result i32 i32) + (i32.const 52) + (i32.const 10)))`, + /calling WebAssembly functions with multiple results from JavaScript not yet implemented/); diff --git a/js/src/jit-test/tests/wasm/multi-value/call-ref.js b/js/src/jit-test/tests/wasm/multi-value/call-ref.js new file mode 100644 index 000000000000..7023a92dc87d --- /dev/null +++ b/js/src/jit-test/tests/wasm/multi-value/call-ref.js @@ -0,0 +1,31 @@ +// |jit-test| skip-if: !wasmReftypesEnabled() + +let counter = 0; +let i = new WebAssembly.Instance( + new WebAssembly.Module(wasmTextToBinary(` + (module + (func $boxNextInt (import "imports" "boxNextInt") + (result anyref)) + (func $unboxInt (import "imports" "unboxInt") + (param anyref) (result i32)) + + (func $boxNextTwoInts (result anyref anyref) + call $boxNextInt + call $boxNextInt) + + (func $unboxTwoInts (param anyref anyref) (result i32 i32) + local.get 0 + call $unboxInt + local.get 1 + call $unboxInt) + + (func $addNextTwoInts (export "addNextTwoInts") (result i32) + call $boxNextTwoInts + call $unboxTwoInts + i32.add))`)), + {imports: {boxNextInt: () => ({val: counter++}), + unboxInt: box => box.val}}); + +for (let n = 0; n < 200000; n += 2) { + assertEq(i.exports.addNextTwoInts(), n * 2 + 1); +} diff --git a/js/src/jit-test/tests/wasm/multi-value/call-run.js b/js/src/jit-test/tests/wasm/multi-value/call-run.js new file mode 100644 index 000000000000..67662a4af529 --- /dev/null +++ b/js/src/jit-test/tests/wasm/multi-value/call-run.js @@ -0,0 +1,120 @@ +wasmFullPass(` + (module + (func (result i32 i32) + (i32.const 52) + (i32.const 10)) + (func (export "run") (result i32) + (call 0) + i32.sub))`, + 42); + +wasmFullPass(` + (module + (func (param i32 i32) (result i32 i32) + (local.get 0) + (local.get 1)) + (func (export "run") (result i32) + (i32.const 52) + (i32.const 10) + (call 0) + i32.sub))`, + 42); + +wasmFullPass(` + (module + (func (param i32 i32 i32 i32 i32 + i32 i32 i32 i32 i32) + (result i32 i32) + (local.get 8) + (local.get 9)) + (func (export "run") (result i32) + (i32.const 0) + (i32.const 1) + (i32.const 2) + (i32.const 3) + (i32.const 4) + (i32.const 5) + (i32.const 6) + (i32.const 7) + (i32.const 52) + (i32.const 10) + (call 0) + i32.sub))`, + 42); + +wasmFullPass(` + (module + (func (param i32 i32 i32 i32 i32 + i32 i32 i32 i32 i32) + (result i32 i32) + (local i32 i32 i32 i32) + (local.get 8) + (local.get 9)) + (func (export "run") (result i32) + (i32.const 0) + (i32.const 1) + (i32.const 2) + (i32.const 3) + (i32.const 4) + (i32.const 5) + (i32.const 6) + (i32.const 7) + (i32.const 52) + (i32.const 10) + (call 0) + i32.sub))`, + 42); + +wasmFullPass(` + (module + (func (param i32 i32 i32 i32 i32 + i32 i32 i32 i32 i32) + (result i32 i32 i32) + (local i32 i32 i32 i32) + (local.get 7) + (local.get 8) + (local.get 9)) + (func (export "run") (result i32) + (i32.const 0) + (i32.const 1) + (i32.const 2) + (i32.const 3) + (i32.const 4) + (i32.const 5) + (i32.const 6) + (i32.const 7) + (i32.const 52) + (i32.const 10) + (call 0) + i32.sub + i32.sub))`, + -35); + +wasmFullPass(` + (module + (func (param i32 i64 i32 i64 i32 + i64 i32 i64 i32 i64) + (result i64 i32 i64) + (local i32 i64 i32 i64) + (local.get 7) + (local.get 8) + (local.get 9)) + (func (export "run") (result i32) + (i32.const 0) + (i64.const 1) + (i32.const 2) + (i64.const 3) + (i32.const 4) + (i64.const 5) + (i32.const 6) + (i64.const 7) + (i32.const 52) + (i64.const 10) + (call 0) + i32.wrap_i64 + i32.sub + i64.extend_i32_s + i64.sub + i32.wrap_i64))`, + -35); + diff --git a/js/src/jit-test/tests/wasm/multi-value/call-validate.js b/js/src/jit-test/tests/wasm/multi-value/call-validate.js new file mode 100644 index 000000000000..513e11bc6e65 --- /dev/null +++ b/js/src/jit-test/tests/wasm/multi-value/call-validate.js @@ -0,0 +1,47 @@ +wasmValidateText(` + (module + (func (result i32 i32) + (i32.const 32) + (i32.const 10)))`); + +wasmValidateText(` + (module + (type $t (func (result i32 i32))) + (func (type $t) + (i32.const 32) + (i32.const 10)))`); + +wasmValidateText(` + (module + (func (result i32 i32) + (block (result i32 i32) + (i32.const 32) + (i32.const 10))))`); + +wasmValidateText(` + (module + (func $return-2 (result i32 i32) + (i32.const 32) + (i32.const 10)) + (func $tail-call (result i32 i32) + (call 0)))`); + +wasmValidateText(` + (module + (func $return-2 (result i32 i32) + (i32.const 32) + (i32.const 10)) + (func $add (result i32) + (call 0) + i32.add))`); + +wasmValidateText(` + (module + (func $return-2 (param i32 i32) (result i32 i32) + (local.get 0) + (local.get 1)) + (func (export "run") (result i32) + (i32.const 32) + (i32.const 10) + (call 0) + i32.add))`); diff --git a/js/src/jit/BacktrackingAllocator.cpp b/js/src/jit/BacktrackingAllocator.cpp index 67800a829afe..3d3d979ee476 100644 --- a/js/src/jit/BacktrackingAllocator.cpp +++ b/js/src/jit/BacktrackingAllocator.cpp @@ -1186,7 +1186,7 @@ void BacktrackingAllocator::allocateStackDefinition(VirtualRegister& reg) { const LUse* use = ins->getOperand(0)->toUse(); VirtualRegister& area = vregs[use->virtualRegister()]; const LStackArea* areaAlloc = area.def()->output()->toStackArea(); - reg.def()->setOutput(areaAlloc->resultAlloc(ins)); + reg.def()->setOutput(areaAlloc->resultAlloc(ins, reg.def())); } } diff --git a/js/src/jit/LIR.h b/js/src/jit/LIR.h index 35ae857b31cc..0e70bf24767c 100644 --- a/js/src/jit/LIR.h +++ b/js/src/jit/LIR.h @@ -35,6 +35,7 @@ class LStackArea; class LArgument; class LConstantIndex; class LInstruction; +class LDefinition; class MBasicBlock; class MIRGenerator; @@ -408,7 +409,7 @@ class LStackArea : public LAllocation { ResultIterator results() const { return ResultIterator(*this); } - inline LStackSlot resultAlloc(LInstruction* lir) const; + inline LStackSlot resultAlloc(LInstruction* lir, LDefinition* def) const; }; // Arguments are reverse indices into the stack. The indices are byte indices. diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index 29bf742ebde7..c1f2cf4d97ab 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -4445,6 +4445,7 @@ void LIRGenerator::visitWasmStackArg(MWasmStackArg* ins) { void LIRGenerator::visitWasmRegisterResult(MWasmRegisterResult* ins) { auto* lir = new (alloc()) LWasmRegisterResult(); uint32_t vreg = getVirtualRegister(); + MOZ_ASSERT(ins->type() != MIRType::Int64); auto type = LDefinition::TypeFrom(ins->type()); lir->setDef(0, LDefinition(vreg, type, LGeneralReg(ins->loc()))); ins->setVirtualRegister(vreg); diff --git a/js/src/jit/shared/LIR-shared.h b/js/src/jit/shared/LIR-shared.h index c2f990f410a7..72c7cd63e543 100644 --- a/js/src/jit/shared/LIR-shared.h +++ b/js/src/jit/shared/LIR-shared.h @@ -6816,6 +6816,13 @@ inline bool LStackArea::ResultIterator::isGcPointer() const { MOZ_ASSERT(!done()); MWasmStackResultArea* area = alloc_.ins()->toWasmStackResultArea()->mir(); MIRType type = area->result(idx_).type(); +#ifndef JS_PUNBOX64 + // LDefinition::TypeFrom isn't defined for MIRType::Int64 values on + // this platform, so here we have a special case. + if (type == MIRType::Int64) { + return false; + } +#endif return LDefinition::TypeFrom(type) == LDefinition::OBJECT; } @@ -6838,15 +6845,28 @@ class LWasmStackResult64 : public LInstructionHelper { LWasmStackResult64() : LInstructionHelper(classOpcode) {} MWasmStackResult* mir() const { return mir_->toWasmStackResult(); } - LStackSlot result(uint32_t base) const { - return LStackSlot(base - mir()->result().offset()); + LStackSlot result(uint32_t base, LDefinition* def) { + uint32_t offset = base - mir()->result().offset(); +#if defined(JS_NUNBOX32) + if (def == getDef(INT64LOW_INDEX)) { + offset -= INT64LOW_OFFSET; + } else { + MOZ_ASSERT(def == getDef(INT64HIGH_INDEX)); + offset -= INT64HIGH_OFFSET; + } +#else + MOZ_ASSERT(def == getDef(0)); +#endif + return LStackSlot(offset); } }; -inline LStackSlot LStackArea::resultAlloc(LInstruction* lir) const { +inline LStackSlot LStackArea::resultAlloc(LInstruction* lir, + LDefinition* def) const { if (lir->isWasmStackResult64()) { - return lir->toWasmStackResult64()->result(base()); + return lir->toWasmStackResult64()->result(base(), def); } + MOZ_ASSERT(def == lir->getDef(0)); return lir->toWasmStackResult()->result(base()); } diff --git a/js/src/js.msg b/js/src/js.msg index 3b323d91cd42..bb5cc6ae5b97 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -429,6 +429,8 @@ MSG_DEF(JSMSG_WASM_TEXT_FAIL, 1, JSEXN_SYNTAXERR, "wasm text error: { MSG_DEF(JSMSG_WASM_MISSING_MAXIMUM, 0, JSEXN_TYPEERR, "'shared' is true but maximum is not specified") MSG_DEF(JSMSG_WASM_GLOBAL_IMMUTABLE, 0, JSEXN_TYPEERR, "can't set value of immutable global") MSG_DEF(JSMSG_WASM_NULL_REQUIRED, 0, JSEXN_TYPEERR, "nullref requires a null value") +MSG_DEF(JSMSG_WASM_MULTIPLE_RESULT_EXIT_UNIMPLEMENTED, 0, JSEXN_TYPEERR, "calling JavaScript functions with multiple results from WebAssembly not yet implemented") +MSG_DEF(JSMSG_WASM_MULTIPLE_RESULT_ENTRY_UNIMPLEMENTED, 0, JSEXN_TYPEERR, "calling WebAssembly functions with multiple results from JavaScript not yet implemented") // Proxy MSG_DEF(JSMSG_BAD_TRAP_RETURN_VALUE, 2, JSEXN_TYPEERR,"trap {1} for {0} returned a primitive value") diff --git a/js/src/wasm/WasmBaselineCompile.cpp b/js/src/wasm/WasmBaselineCompile.cpp index 422c686b79a3..14d9744e8427 100644 --- a/js/src/wasm/WasmBaselineCompile.cpp +++ b/js/src/wasm/WasmBaselineCompile.cpp @@ -1738,7 +1738,7 @@ class BaseStackFrame final : public BaseStackFrameAllocator { RegPtr dest) { MOZ_ASSERT(results.height() <= masm.framePushed()); uint32_t offsetFromSP = masm.framePushed() - results.height(); - masm.movePtr(AsRegister(sp_), dest); + masm.moveStackPtrTo(dest); if (offsetFromSP) { masm.addPtr(Imm32(offsetFromSP), dest); } diff --git a/js/src/wasm/WasmCode.h b/js/src/wasm/WasmCode.h index b3fcb3752359..8a6828125363 100644 --- a/js/src/wasm/WasmCode.h +++ b/js/src/wasm/WasmCode.h @@ -239,6 +239,7 @@ class FuncExport { bool canHaveJitEntry() const { return !funcType_.temporarilyUnsupportedReftypeForEntry() && + !funcType_.temporarilyUnsupportedResultCountForEntry() && JitOptions.enableWasmJitEntry; } diff --git a/js/src/wasm/WasmConstants.h b/js/src/wasm/WasmConstants.h index 4c2600f5b3f3..37c1a61a410a 100644 --- a/js/src/wasm/WasmConstants.h +++ b/js/src/wasm/WasmConstants.h @@ -603,11 +603,6 @@ static const unsigned MaxBrTableElems = 1000000; static const unsigned MaxMemoryInitialPages = 16384; static const unsigned MaxCodeSectionBytes = MaxModuleBytes; -// FIXME: Temporary limit to function result counts. Replace with MaxResults: -// bug 1585909. - -static const unsigned MaxFuncResults = 1; - // A magic value of the FramePointer to indicate after a return to the entry // stub that an exception has been caught and that we should throw. diff --git a/js/src/wasm/WasmInstance.cpp b/js/src/wasm/WasmInstance.cpp index 4b7f7b9b3447..8364d4a7eb3b 100644 --- a/js/src/wasm/WasmInstance.cpp +++ b/js/src/wasm/WasmInstance.cpp @@ -123,6 +123,12 @@ bool Instance::callImport(JSContext* cx, uint32_t funcImportIndex, return false; } + if (fi.funcType().temporarilyUnsupportedResultCountForExit()) { + JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr, + JSMSG_WASM_MULTIPLE_RESULT_EXIT_UNIMPLEMENTED); + return false; + } + MOZ_ASSERT(fi.funcType().args().length() == argc); for (size_t i = 0; i < argc; i++) { switch (fi.funcType().args()[i].kind()) { @@ -1698,6 +1704,12 @@ bool Instance::callExport(JSContext* cx, uint32_t funcIndex, CallArgs args) { return false; } + if (funcType->temporarilyUnsupportedResultCountForEntry()) { + JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr, + JSMSG_WASM_MULTIPLE_RESULT_ENTRY_UNIMPLEMENTED); + return false; + } + // The calling convention for an external call into wasm is to pass an // array of 16-byte values where each value contains either a coerced int32 // (in the low word), or a double value (in the low dword) value, with the diff --git a/js/src/wasm/WasmOpIter.h b/js/src/wasm/WasmOpIter.h index 6935032cf6ec..bd436a968aed 100644 --- a/js/src/wasm/WasmOpIter.h +++ b/js/src/wasm/WasmOpIter.h @@ -2061,12 +2061,6 @@ inline bool OpIter::readCallIndirect(uint32_t* funcTypeIndex, const FuncType& funcType = env_.types[*funcTypeIndex].funcType(); - // FIXME: Remove this check when full multi-value function returns land. - // Bug 1585909. - if (funcType.results().length() > MaxFuncResults) { - return fail("too many returns in signature"); - } - #ifdef WASM_PRIVATE_REFTYPES if (env_.tables[*tableIndex].importedOrExported && funcType.exposesTypeIndex()) { @@ -2131,12 +2125,6 @@ inline bool OpIter::readOldCallIndirect(uint32_t* funcTypeIndex, const FuncType& funcType = env_.types[*funcTypeIndex].funcType(); - // FIXME: Remove this check when full multi-value function returns land. - // Bug 1585909. - if (funcType.results().length() > MaxFuncResults) { - return fail("too many returns in signature"); - } - if (!popCallArgs(funcType.args(), argValues)) { return false; } diff --git a/js/src/wasm/WasmStubs.cpp b/js/src/wasm/WasmStubs.cpp index 3b3b8701b783..9dc9881c931c 100644 --- a/js/src/wasm/WasmStubs.cpp +++ b/js/src/wasm/WasmStubs.cpp @@ -641,6 +641,12 @@ static bool GenerateInterpEntry(MacroAssembler& masm, const FuncExport& fe, # endif #endif + if (fe.funcType().temporarilyUnsupportedResultCountForEntry()) { + // Unreachable as the Instance::callExport doesn't let us get here. + masm.breakpoint(); + return FinishOffsets(masm, offsets); + } + // Save all caller non-volatile registers before we clobber them here and in // the wasm callee (which does not preserve non-volatile registers). masm.setFramePushed(0); @@ -1566,7 +1572,8 @@ static void StackCopy(MacroAssembler& masm, MIRType type, Register scratch, GenPrintIsize(DebugChannel::Import, masm, scratch); masm.store64(scratch64, dst); #endif - } else if (type == MIRType::RefOrNull || type == MIRType::Pointer) { + } else if (type == MIRType::RefOrNull || type == MIRType::Pointer || + type == MIRType::StackResults) { masm.loadPtr(src, scratch); GenPrintPtr(DebugChannel::Import, masm, scratch); masm.storePtr(scratch, dst); @@ -1606,13 +1613,11 @@ static void FillArgumentArrayForExit( ArgTypeVector args(funcType); for (ABIArgIter i(args); !i.done(); i++) { - if (args.isSyntheticStackResultPointerArg(i.index())) { - MOZ_CRASH("Exit to function returning multiple values unimplemented"); - } - Address dst(masm.getStackPointer(), argOffset + i.index() * sizeof(Value)); MIRType type = i.mirType(); + MOZ_ASSERT(args.isSyntheticStackResultPointerArg(i.index()) == + (type == MIRType::StackResults)); switch (i->kind()) { case ABIArg::GPR: if (type == MIRType::Int32) { @@ -1648,6 +1653,10 @@ static void FillArgumentArrayForExit( GenPrintPtr(DebugChannel::Import, masm, i->gpr()); masm.storePtr(i->gpr(), dst); } + } else if (type == MIRType::StackResults) { + MOZ_ASSERT(!toValue, "Multi-result exit to JIT unimplemented"); + GenPrintPtr(DebugChannel::Import, masm, i->gpr()); + masm.storePtr(i->gpr(), dst); } else { MOZ_CRASH("FillArgumentArrayForExit, ABIArg::GPR: unexpected type"); } @@ -1878,8 +1887,9 @@ static bool GenerateImportInterpExit(MacroAssembler& masm, const FuncImport& fi, // padding between argv and retaddr ensures that sp is aligned. unsigned argOffset = AlignBytes(StackArgBytes(invokeArgTypes), sizeof(double)); - unsigned argBytes = - std::max(1, fi.funcType().args().length()) * sizeof(Value); + // The abiArgCount includes a stack result pointer argument if needed. + unsigned abiArgCount = ArgTypeVector(fi.funcType()).length(); + unsigned argBytes = std::max(1, abiArgCount * sizeof(Value)); unsigned framePushed = StackDecrementForCall(ABIStackAlignment, sizeof(Frame), // pushed by prologue @@ -1946,16 +1956,22 @@ static bool GenerateImportInterpExit(MacroAssembler& masm, const FuncImport& fi, // Make the call, test whether it succeeded, and extract the return value. AssertStackAlignment(masm, ABIStackAlignment); - const ValTypeVector& results = fi.funcType().results(); - if (results.length() == 0) { + ResultType resultType = ResultType::Vector(fi.funcType().results()); + ValType registerResultType; + for (ABIResultIter iter(resultType); !iter.done(); iter.next()) { + if (iter.cur().inRegister()) { + MOZ_ASSERT(!registerResultType.isValid()); + registerResultType = iter.cur().type(); + } + } + if (!registerResultType.isValid()) { masm.call(SymbolicAddress::CallImport_Void); masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, throwLabel); GenPrintf(DebugChannel::Import, masm, "wasm-import[%u]; returns ", funcImportIndex); GenPrintf(DebugChannel::Import, masm, "void"); } else { - MOZ_ASSERT(results.length() == 1, "multi-value return unimplemented"); - switch (results[0].kind()) { + switch (registerResultType.kind()) { case ValType::I32: masm.call(SymbolicAddress::CallImport_I32); masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, throwLabel); @@ -1990,7 +2006,7 @@ static bool GenerateImportInterpExit(MacroAssembler& masm, const FuncImport& fi, GenPrintF64(DebugChannel::Import, masm, ReturnDoubleReg); break; case ValType::Ref: - switch (results[0].refTypeKind()) { + switch (registerResultType.refTypeKind()) { case RefType::Func: masm.call(SymbolicAddress::CallImport_FuncRef); masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, @@ -2693,6 +2709,11 @@ bool wasm::GenerateEntryStubs(MacroAssembler& masm, size_t funcExportIndex, return true; } + // Returning multiple values to JS not yet implemented (see bug 1595031). + if (fe.funcType().temporarilyUnsupportedResultCountForEntry()) { + return true; + } + if (!GenerateJitEntry(masm, funcExportIndex, fe, callee, bigIntEnabled, &offsets)) { return false; @@ -2737,6 +2758,12 @@ bool wasm::GenerateStubs(const ModuleEnvironment& env, continue; } + // Exit to JS returning multiple values not yet implemented (see bug + // 1595031). + if (fi.funcType().temporarilyUnsupportedResultCountForExit()) { + continue; + } + JitExitOffsets jitOffsets; if (!GenerateImportJitExit(masm, fi, funcIndex, &throwLabel, &jitOffsets)) { return false; diff --git a/js/src/wasm/WasmTextToBinary.cpp b/js/src/wasm/WasmTextToBinary.cpp index 652e6d985b1e..3aee2862cd5c 100644 --- a/js/src/wasm/WasmTextToBinary.cpp +++ b/js/src/wasm/WasmTextToBinary.cpp @@ -2464,15 +2464,11 @@ static bool ParseFuncSig(WasmParseContext& c, AstFuncType* funcType, return true; } -// For ParseAnonFuncType: allow more than one results. -enum class MultiResult { False, True }; - // This guarantees that the ref has an index when we return and won't need to be // resolved later. If `withLookahead` then whatever comes after the type does // not cause an error and is not consumed, and any number of results are // allowed. static bool ParseAnonFuncType(WasmParseContext& c, AstRef* ref, - MultiResult multiResult, WithLookahead withLookahead) { MOZ_ASSERT(ref->isInvalid()); @@ -2480,10 +2476,6 @@ static bool ParseAnonFuncType(WasmParseContext& c, AstRef* ref, if (!ParseFuncSig(c, &funcType, withLookahead)) { return false; } - if (multiResult == MultiResult::False && funcType.results().length() > 1) { - c.ts.generateError(c.ts.peek(), "too many results", c.error); - return false; - } uint32_t funcTypeIndex; if (!c.module->declare(std::move(funcType), &funcTypeIndex)) { return false; @@ -2525,7 +2517,7 @@ static bool ParseBlockType(WasmParseContext& c, AstBlockType* type) { } AstRef t; - if (!ParseAnonFuncType(c, &t, MultiResult::True, WithLookahead::True)) { + if (!ParseAnonFuncType(c, &t, WithLookahead::True)) { return false; } const AstFuncType& ft = c.module->types()[t.index()]->asFuncType(); @@ -4202,7 +4194,7 @@ static bool ParseFuncType(WasmParseContext& c, AstRef* ref) { return false; } if (ref->isInvalid()) { - if (!ParseAnonFuncType(c, ref, MultiResult::False, WithLookahead::False)) { + if (!ParseAnonFuncType(c, ref, WithLookahead::False)) { return false; } } @@ -4289,10 +4281,6 @@ static bool ParseFunc(WasmParseContext& c) { if (!ParseValueTypeList(c, &results)) { return false; } - if (results.length() > 1) { - c.ts.generateError(token, "too many results", c.error); - return false; - } break; default: c.ts.unget(token); @@ -4382,10 +4370,6 @@ static AstTypeDef* ParseTypeDef(WasmParseContext& c) { if (!ParseFuncSig(c, &funcType, WithLookahead::False)) { return nullptr; } - if (funcType.results().length() > 1) { - c.ts.generateError(c.ts.peek(), "too many results", c.error); - return nullptr; - } type = new (c.lifo) AstFuncType(name, std::move(funcType)); } else if (c.ts.getIf(WasmToken::Struct)) { AstStructType st(c.lifo); @@ -4801,8 +4785,7 @@ static AstImport* ParseImport(WasmParseContext& c) { } if (funcTypeRef.isInvalid()) { - if (!ParseAnonFuncType(c, &funcTypeRef, MultiResult::False, - WithLookahead::False)) { + if (!ParseAnonFuncType(c, &funcTypeRef, WithLookahead::False)) { return nullptr; } } diff --git a/js/src/wasm/WasmTypes.h b/js/src/wasm/WasmTypes.h index 916c2b508d40..b2fce8aa2951 100644 --- a/js/src/wasm/WasmTypes.h +++ b/js/src/wasm/WasmTypes.h @@ -1086,6 +1086,16 @@ class FuncType { } return false; } + // Entry from JS to wasm is currently unimplemented for functions that return + // multiple values. + bool temporarilyUnsupportedResultCountForEntry() const { + return results().length() > 1; + } + // Calls out from wasm to JS that return multiple values is currently + // unsupported. + bool temporarilyUnsupportedResultCountForExit() const { + return results().length() > 1; + } // For JS->wasm jit entries, AnyRef parameters and returns are allowed, // as are all reference types apart from TypeIndex. bool temporarilyUnsupportedReftypeForEntry() const { diff --git a/js/src/wasm/WasmValidate.cpp b/js/src/wasm/WasmValidate.cpp index a7d99a846f0a..6a5618e16e31 100644 --- a/js/src/wasm/WasmValidate.cpp +++ b/js/src/wasm/WasmValidate.cpp @@ -1214,8 +1214,6 @@ static bool DecodeFunctionBodyExprs(const ModuleEnvironment& env, bool wasm::ValidateFunctionBody(const ModuleEnvironment& env, uint32_t funcIndex, uint32_t bodySize, Decoder& d) { - MOZ_ASSERT(env.funcTypes[funcIndex]->results().length() <= MaxFuncResults); - ValTypeVector locals; if (!locals.appendAll(env.funcTypes[funcIndex]->args())) { return false; @@ -1590,12 +1588,6 @@ static bool DecodeSignatureIndex(Decoder& d, const TypeDefVector& types, return d.fail("signature index references non-signature"); } - // FIXME: Remove this check when full multi-value function returns land. - // Bug 1585909. - if (def.funcType().results().length() > MaxFuncResults) { - return d.fail("too many returns in signature"); - } - return true; } diff --git a/testing/web-platform/meta/wasm/jsapi/constructor/multi-value.any.js.ini b/testing/web-platform/meta/wasm/jsapi/constructor/multi-value.any.js.ini index 12893b2bf99a..1ef3fb5cc483 100644 --- a/testing/web-platform/meta/wasm/jsapi/constructor/multi-value.any.js.ini +++ b/testing/web-platform/meta/wasm/jsapi/constructor/multi-value.any.js.ini @@ -14,7 +14,8 @@ expected: FAIL [multiple return values inside wasm] - expected: FAIL + expected: + if not nightly_build: FAIL [multiple return values from js to wasm] expected: FAIL