Conversation
|
Cool! I'll look at removing the extraneous |
|
I think the extra |
|
It looks like this is all that's needed: diff --git a/src/wasm2asm.h b/src/wasm2asm.h
index 59f2d6a..5cd757e 100644
--- a/src/wasm2asm.h
+++ b/src/wasm2asm.h
@@ -655,7 +655,8 @@ Ref Wasm2AsmBuilder::processFunction(Function* func) {
ExpressionList* stats = isBodyBlock ?
&static_cast<Block*>(func->body)->list : nullptr;
bool endsInReturn =
- (isBodyBlock && ((*stats)[stats->size()-1]->_id == Expression::ReturnId));
+ (isBodyBlock && ((*stats)[stats->size()-1]->_id == Expression::ReturnId)) ||
+ func->body->_id == Expression::ReturnId;
if (endsInReturn) {
// return already taken care of
flattenAppend(ret, processFunctionBody(func, NO_RESULT));which requires regenerating all the |
Just wanted to make sure you're aware of |
I was not, that's super useful! I had written a one-liner to update the tests, but it wasn't perfect; having an already-written script will make things much easier. |
This change eliminates one issue that prevents asm.js validation of the generated code, see WebAssembly#1443.
This change eliminates one issue that prevents asm.js validation of the generated code, see WebAssembly#1443.
This change eliminates one issue that prevents asm.js validation of the generated code, see WebAssembly#1443.
This change eliminates one issue that prevents asm.js validation of the generated code, see WebAssembly#1443.
This change eliminates one issue that prevents asm.js validation of the generated code, see WebAssembly#1443.
This change eliminates one issue that prevents asm.js validation of the generated code, see WebAssembly#1443.
This change eliminates one issue that prevents asm.js validation of the generated code, see #1443.
|
@kripken Can you repush this to re-run CI now that multiple return statements have been fixed? |
|
Done. |
We were using Math_{min,max} in wasm2asm-generated files without
declaring said functions. This decision created problems for tests,
because Math_min (resp. max) would first be used on f32s, thus returning
f32, and then validation would fail when it was used on f64s.
The resulting changes make wasm2asm tests pass with MOZJS asm.js
validation, which moves WebAssembly#1443 forward.
We were using Math_{min,max} in wasm2asm-generated files without
declaring said functions. This decision created problems for tests,
because Math_min (resp. max) would first be used on f32s, thus returning
f32, and then validation would fail when it was used on f64s.
The resulting changes make wasm2asm tests pass with MOZJS asm.js
validation, which moves WebAssembly#1443 forward.
We were using Math_{min,max} in wasm2asm-generated files without
declaring said functions. This decision created problems for tests,
because Math_min (resp. max) would first be used on f32s, thus returning
f32, and then validation would fail when it was used on f64s.
The resulting changes make wasm2asm tests pass with MOZJS asm.js
validation, which moves #1443 forward.
|
Great, after your last fixes @froydnj this PR is now green! Any concerns or should I land this? |
|
I would have said mentioning testing with node and spidermonkey in the |
jsvu is an easy way to get JS VMs. This uses it to get SpiderMonkey, which is then used to check asm.js validation in the wasm2asm tests.
Those currently fail, so we should merge this only after those are fixed. We should probably make fixing those a priority. cc @froydnj