do not use ftCall in wasm.

[waterlike86]

instead, just use dynCalls normally in asm.js, each module has a table, so we need a special mechanism to call between them all; in wasm, they all share a single Table
this PR combines work from https://github.com/emscripten-core/emscripten/compare/no-ftCall-in-wasm
and #8039 into 1
see issue #7679

[rianhunter]

Does this change the ABI between the WASM and the JS runtime? i.e. Will wasm files produced with this change not work with the JS support files produced before this change? If not, could you increment EMSCRIPTEN_API_MINOR?

[kripken]

About the tests failing on mftCall_* is not defined - perhaps we have a place where we have mftCall_* which for wasm should be just dynCall_*.

[waterlike86]

@kripken so the code generated is as such

function dynCall_v(index) {
 index = index | 0;
 mftCall_v(index);
}

but it should be generating

function dynCall_v(index) {
 index = index | 0;
 FUNCTION_TABLE_V...
}

which eventually gets translated to

function dynCall_v(index) {
 index = index | 0;
 Module["asm"][index]()
}

does this look right?

[waterlike86]

the problem comes when we use WASM and EMULATED_FUNCTION_POINTERS together, i.e. dynamic linking. it generates the mftCall_X call but does not translate it to an actual table call in the later passes.

[waterlike86]

seems like after my fix there are tests now which says jsCall_ii is not defined.
@kripken could you please tag the lastest binaryen with your legalizeJS fixes so i can include it here? that should fix the no_legalizeffi test.

[kripken]

Yes, what you wrote there sounds right to me.

i tagged version_68 now in binaryen, and opened #8093 . (We should land that before this, but you can merge it in here for now for testing.)

[waterlike86]

Yes, what you wrote there sounds right to me.

i tagged version_68 now in binaryen, and opened #8093 . (We should land that before this, but you can merge it in here for now for testing.)

thanks @kripken I merged it

[waterlike86]

@kripken i believe i fixed most of the tests except test_binaryen_metadce_. does the test need rebasing?

[kripken]

Yes, that test needs to be updated (in PRs that change the number of imports or exports to the programs it tracks). To do that, you would basically just run the failing test locally and see the value it emits, and update the test accordingly (in this PR, these changes are expected as it is adding dynCalls to the module). I did that now, so you can just apply this diff:

diff --git a/tests/test_other.py b/tests/test_other.py
index 687890789..6d617f4ad 100644
--- a/tests/test_other.py
+++ b/tests/test_other.py
@@ -7976,7 +7976,7 @@ int main() {
       self.run_metadce_tests(path_from_root('tests', 'hello_libcxx.cpp'), [
         (['-O2'], 34, ['abort'], ['waka'], 196709,  28,   37, 660), # noqa
         (['-O2', '-s', 'EMULATED_FUNCTION_POINTERS=1'],
-                  34, ['abort'], ['waka'], 196709,  28,   18, 621), # noqa
+                  34, ['abort'], ['waka'], 196709,  28,   38, 642), # noqa
       ]) # noqa
 
   def test_binaryen_metadce_hello(self):
@@ -8005,7 +8005,7 @@ int main() {
                    0, [],        [],           8,   0,    0,  0), # noqa; totally empty!
         # we don't metadce with linkable code! other modules may want stuff
         (['-O3', '-s', 'MAIN_MODULE=1'],
-                1533, [],        [],      226057,  28,   85, None), # noqa; don't compare the # of functions in a main module, which changes a lot
+                1533, [],        [],      226403,  28,   93, None), # noqa; don't compare the # of functions in a main module, which changes a lot
       ]) # noqa
 
   # ensures runtime exports work, even with metadce

[kripken]

Please also add yourself to AUTHORS. Thanks!

[waterlike86]

Thanks @kripken the tests passed now.

[kripken]

Great, thanks @waterlike86!

[awtcode]

Thanks for merging this @kripken :) Can you release a new emsdk with this PR so that we can consume it?

[kripken]

Sure, tagged 1.38.28 now. Should be binaries in a few hours on the bots.

[awtcode]

Thanks @kripken :) We will test it out asap.