don't legalize f32 calls when emulated function pointers

[kripken]

The emulated table is ok either way, it doesn't need legal calls like real wasm calls do.

This may fix emscripten-core/emscripten#5436, but should be tested carefully before landing.

[kripken]

Looks like this breaks binaryen*.test_funcptr_import_type.

[Beuc]

Hi,

I can't get the testsuite to fail:

$ EMTEST_ALL_ENGINES=1 python runner.py binaryen*.test_funcptr_import_type
(using ALL js engines)
Test suites:
['test_core']
Running test_core: (6 tests)
test_funcptr_import_type (test_core.binaryen2) ... ok
test_funcptr_import_type (test_core.binaryens) ... ok
test_funcptr_import_type (test_core.binaryenz) ... ok
test_funcptr_import_type (test_core.binaryen1) ... ok
test_funcptr_import_type (test_core.binaryen3) ... ok
test_funcptr_import_type (test_core.binaryen0) ... ok

DONE: combining results on main thread

test_funcptr_import_type (test_core.binaryen0) ... ok
test_funcptr_import_type (test_core.binaryen1) ... ok
test_funcptr_import_type (test_core.binaryen2) ... ok
test_funcptr_import_type (test_core.binaryen3) ... ok
test_funcptr_import_type (test_core.binaryens) ... ok
test_funcptr_import_type (test_core.binaryenz) ... ok

----------------------------------------------------------------------
Ran 6 tests in 5.674s

OK

I get this result with and without patching myfastcomp.
Am I doing wrong, or was the bug fixed independently? :)

[kripken]

I don't recall a recent fix for this, but maybe I forgot...

Where is that test, btw? Trying to run it on incoming, I don't see it.

[Beuc]

The test is the one you referenced just above, it's in emscripten (not in fastcomp).

.../emscripten-incoming/tests$ EMTEST_ALL_ENGINES=1 python runner.py binaryen*.test_funcptr_import_type

There's a recent commit in fastcomp 1af410b that might be related, but I didn't check.

[kripken]

Thanks @Beuc, I think I had a typo or something before... not sure how I didn't find it...

Testing it now, it does pass in the main test suite, but if you set ``EMULATED_FUNCTION_POINTERS`, which is the relevant mode for this, then it fails. I actually see it fail that way even before this PR, but we should get that passing before landing this, as it's directly relevant - and I'm not sure if we have another test that covers this.

[Beuc]

(it took me a few tries to type it correctly as well, haha)

Got it - one needs to modify EMULATED_FUNCTION_POINTERS in src/settings.js before running the test.
Indeed without the patch bynaryen0-1 fail, while with the patch all bynaryen0-6 fail.

[Beuc]

The test failure looks like emscripten/#6759 - float parameters currently don't work well in WASM+EMULATED_FUNCTION_POINTER_CASTS.
The test complains about the float parameters being 0, which is consistent.
EDIT: scrap that, this was related to EMTERPRETIFY.

[Beuc]

Running the test directly:

$ emcc test_funcptr_import_type.cpp --js-library test_funcptr_import_type.js -o t/test.js -s EMULATE_FUNCTION_POINTER_CASTS=1 -s WASM=1; node t/test.js

Without this PR applied, if I patch-up the generated test.js like this:

-var FUNCTION_TABLE_X = [b0,asm['___stdio_close'],asm['___stdout_write'],asm['___stdio_seek'],_floaty$legalf32,_floatyAlone$legalf32,asm['___stdio_write'],b0];
+var FUNCTION_TABLE_X = [b0,asm['___stdio_close'],asm['___stdout_write'],asm['___stdio_seek'],_floaty,_floatyAlone,asm['___stdio_write'],b0];

or even like this:

-var FUNCTION_TABLE_X = [b0,asm['___stdio_close'],asm['___stdout_write'],asm['___stdio_seek'],_floaty$legalf32,_floatyAlone$legalf32,asm['___stdio_write'],b0];
+var FUNCTION_TABLE_X = [b0,asm['___stdio_close'],asm['___stdout_write'],asm['___stdio_seek'],asm['___stdio_write'],b0];

I get the expected behavior for test_funcptr_import_type, and no "_floaty$legalf32 is not defined" error anymore.

With this PR applied, the generated JS is like my first patch-up, but the test.wasm is vastly different.
I'm not sure if this .wasm change was intended.

[kripken]

This issue may go away thanks to emscripten-core/emscripten#7656 , as it changes how we do emulated function pointers.

If not, it should also just work in the LLVM wasm backend, which we hope to suggest people switch to soon (as that doesn't have any special emulated function pointers mode).

[Beuc]

Is the LLVM wasm backend still incompatible with Emterpreter?
I would need both EMULATE_FUNCTION_POINTER_CASTS and Emterpreter :)

[kripken]

EMULATE_FUNCTION_POINTER_CASTS works with the wasm backend. The emterpreter doesn't yet, but I intend to get that working through wasm2js, which is making good progress.