-s WASM=1 not working with emterpreter

[aidanhs]

The full command is pretty long, but in short I'm trying to create an async emterpreter with wasm. It works fine with asm.js, but fails with -s WASM=1. I've tried applying #5017 just on the off-chance with no success. I'm on incoming and have tested with both ayzim and the normal emscripten optimizer with identical behaviour.

The problem comes from js-optimizer.js, the throw 'bad'; in these lines:

        switch (asmData.ret) {
          case ASM_INT: ret = srcToExp('HEAP32[EMTSTACKTOP >> 2]'); break;
          case ASM_FLOAT:
          case ASM_DOUBLE: ret = srcToExp('HEAPF64[EMTSTACKTOP >> 3]'); break;
          default: throw 'bad';
        }

Modifying the default line to read default: { console.log('going bad'); console.log(asmData.ret); console.log(func); console.log('gone bad'); throw 'bad'; }, I get the following output:

going bad
12
[ 'defun',
  '_combine_soft_light_c',
  [ 'f1', 'f3', 'f4', 'f5' ],
  [ [ 'if', [Object], [Object] ], [ 'stat', [Object] ] ] ]
gone bad

Note that 12 corresponds to the ASM_NONE type.

Some thoughts: when creating ayzim, I was able to remove ASM_NONE and make the type of asmData.ret be an Option<AsmType>, i.e. ASM_NONE is more accurately expressed as the absence of a type. Making an educated guess based on this knowledge, I changed if (asmData.ret !== undefined) { to if (asmData.ret !== undefined && asmData.ret !== ASM_NONE) { a couple of lines above this switch (as well as a similar check elsewhere), which then allowed the emterpreter step to complete. Sadly, it later failed in the asm2wasm step so I guess my changes may have just been putting off the inevitable crash:

DEBUG:root:check tells us to use asm.js backend
DEBUG:root:using binaryen, with method: native-wasm
DEBUG:root:asm2wasm (asm.js => WebAssembly): /myhome/.emscripten_ports/binaryen/binaryen-version_30/bin/asm2wasm out/wesnoth.temp.asm.js --total-memory=1073741824 --emit-jsified-potential-traps -O3 --mem-init=out/wesnoth.html.mem --mem-base=1024 -o out/wesnoth.wasm
asm2wasm: /myhome/.emscripten_ports/binaryen/binaryen-version_30/src/wasm/wasm-binary.cpp:320: uint32_t wasm::WasmBinaryWriter::getFunctionIndex(wasm::Name): Assertion `mappedFunctions.count(name)' failed.

[aidanhs]

Oh but hang on, does a wasm module need to be fully linked? I.e. is it unable to handle

warning: unresolved symbol: posix_spawn_file_actions_init
warning: unresolved symbol: BZ2_bzDecompressInit
warning: unresolved symbol: posix_spawn_file_actions_destroy
warning: unresolved symbol: BZ2_bzDecompressEnd
warning: unresolved symbol: BZ2_bzCompress
warning: unresolved symbol: BZ2_bzCompressInit
warning: unresolved symbol: sendfile
warning: unresolved symbol: BZ2_bzDecompress
warning: unresolved symbol: BZ2_bzCompressEnd
warning: unresolved symbol: posix_spawn_file_actions_adddup2

?

If so, I guess the two errors are unrelated.

[kripken]

Yeah, the errors seem to be unrelated, as best I can guess.

As to the first error, that's odd. We do set asmData.ret to undefined, and only set it to anything else if we find a return statement with a value (looking at all sites with .ret = in that file). So asmData.ret should never be ASM_NONE. That's worth investigating - it's an unknown bug, not detected by the test suite nor fuzzing.

Also interesting that wasm influences things here. It actually shouldn't at this stage, this should be the same asm.js as when building the same file to asm.js.

Anyhow, maybe if you just add some debug logging after the one place we do .ret = in that file (check for assigning ASM_NONE which is invalid, and printing the function), the problem might be come obvious. If not, maybe send me the bitcode + build command.

[aidanhs]

My link command involves 100MB across 15 different bitcode files, so might be a little inconvenient to send - I'll have a go at it :)

[aidanhs]

Ah but i can combine files if necessary, so that's a possibility.

[kripken]

Yeah, if you can combine them that's fine, if the logging suggested above doesn't turn out something obvious for you.

[aidanhs]

Got it. See #5046.

[aidanhs]

Oh but hang on, does a wasm module need to be fully linked?

Now I've got a successful wasm build, the answer appears to be a definite no - it works fine. So it seems that the second problem was resolved by something else, possibly updating incoming, possibly the PR above. Not important any more.

[aidanhs]

Aha, got the binaryen assertion again. With debugging:

count _emscripten_glFrustum
asm2wasm: /myhome/.emscripten_ports/binaryen/binaryen-version_31/src/wasm/wasm-binary.cpp:324: uint32_t wasm::WasmBinaryWriter::getFunctionIndex(wasm::Name): Assertion `mappedFunctions.count(name)' failed.

i.e. the function it can't find is _emscripten_glFrustum. Grepping the file for that:

$ grep emscripten_glFrustum out/wesnoth.temp.asm.js
  var _emscripten_glFrustum=env._emscripten_glFrustum;
,b21,b21,b21,_emscripten_glFrustum,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21,b21

[kripken]

Hmm, so it's imported and used only in a table. That should be fine, but we did fix bugs there - which version are you on?

[aidanhs]

I was using incoming as of the merge of my asm float zero fix. Two commits before that was "update binaryen port to version_31". So pretty much the latest.

After updating and wiping my cache, I now can't reproduce again. I did have a number of PRs ongoing branched off from different points that could have messed things up, so I'll close this issue since everything I was originally complaining about now :)