wasm-emscripten-finalize: Remove JSCall thunk generation (#1938)

We now implement addFunction by creating a wasm module to wrap
that JS function and simply adding it to the table.

7 files changed, 7 insertions, 498 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 36abe6199..67fa19c82 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,9 @@ full changeset diff at the end of each section.
 Current Trunk
 -------------
 
+- Remove jsCall generation from wasm-emscripten-finalize.  This is not needed
+  as of https://github.com/emscripten-core/emscripten/pull/8255.
+
 ### BREAKING CHANGES (old to new)
 
 v.55
diff --git a/auto_update_tests.py b/auto_update_tests.py
index f4b13c737..7157e58be 100755
--- a/auto_update_tests.py
+++ b/auto_update_tests.py
@@ -79,7 +79,6 @@ def update_lld_tests():
     mem_file = wast_path + '.mem'
     extension_arg_map = {
       '.out': [],
-      '.jscall.out': ['--emscripten-reserved-function-pointers=3'],
       '.mem.out': ['--separate-data-segments', mem_file + '.mem'],
     }
     for ext, ext_args in extension_arg_map.items():
diff --git a/scripts/test/lld.py b/scripts/test/lld.py
index 03c96d6c3..94736f03f 100755
--- a/scripts/test/lld.py
+++ b/scripts/test/lld.py
@@ -29,7 +29,6 @@ def test_wasm_emscripten_finalize():
     mem_file = wast_path + '.mem'
     extension_arg_map = {
       '.out': [],
-      '.jscall.out': ['--emscripten-reserved-function-pointers=3'],
       '.mem.out': ['--separate-data-segments', mem_file],
     }
     for ext, ext_args in extension_arg_map.items():
diff --git a/src/tools/wasm-emscripten-finalize.cpp b/src/tools/wasm-emscripten-finalize.cpp
index 42fc1e4db..6e898b3f0 100644
--- a/src/tools/wasm-emscripten-finalize.cpp
+++ b/src/tools/wasm-emscripten-finalize.cpp
@@ -47,7 +47,6 @@ int main(int argc, const char *argv[]) {
   bool emitBinary = true;
   bool debugInfo = false;
   bool legalizeJavaScriptFFI = true;
-  unsigned numReservedFunctionPointers = 0;
   uint64_t globalBase = INVALID_BASE;
   uint64_t initialStackPointer = INVALID_BASE;
   Options options("wasm-emscripten-finalize",
@@ -70,14 +69,6 @@ int main(int argc, const char *argv[]) {
            [&emitBinary](Options*, const std::string& ) {
              emitBinary = false;
            })
-      .add("--emscripten-reserved-function-pointers", "",
-           "Number of reserved function pointers for emscripten addFunction "
-           "support",
-           Options::Arguments::One,
-           [&numReservedFunctionPointers](Options *,
-                                          const std::string &argument) {
-             numReservedFunctionPointers = std::stoi(argument);
-           })
       .add("--global-base", "", "The address at which static globals were placed",
            Options::Arguments::One,
            [&globalBase](Options*, const std::string&argument ) {
@@ -203,7 +194,6 @@ int main(int argc, const char *argv[]) {
   }
 
   generator.generateDynCallThunks();
-  generator.generateJSCallThunks(numReservedFunctionPointers);
 
   // Legalize the wasm.
   {
@@ -218,7 +208,7 @@ int main(int argc, const char *argv[]) {
   }
 
   // Substantial changes to the wasm are done, enough to create the metadata.
-  std::string metadata = generator.generateEmscriptenMetadata(dataSize, initializerFunctions, numReservedFunctionPointers);
+  std::string metadata = generator.generateEmscriptenMetadata(dataSize, initializerFunctions);
 
   // Finally, separate out data segments if relevant (they may have been needed
   // for metadata).
diff --git a/src/wasm-emscripten.h b/src/wasm-emscripten.h
index 275c809fd..acb2994ad 100644
--- a/src/wasm-emscripten.h
+++ b/src/wasm-emscripten.h
@@ -46,14 +46,8 @@ public:
   // and restore functions.
   void replaceStackPointerGlobal();
 
-  // Create thunks to support emscripten's addFunction functionality. Creates (#
-  // of reserved function pointers) thunks for each indirectly called function
-  // signature.
-  void generateJSCallThunks(unsigned numReservedFunctionPointers);
-
   std::string generateEmscriptenMetadata(
-      Address staticBump, std::vector<Name> const& initializerFunctions,
-      unsigned numReservedFunctionPointers);
+    Address staticBump, std::vector<Name> const& initializerFunctions);
 
   void fixInvokeFunctionNames();
 
diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp
index 390266d44..a21c45476 100644
--- a/src/wasm/wasm-emscripten.cpp
+++ b/src/wasm/wasm-emscripten.cpp
@@ -270,109 +270,6 @@ void EmscriptenGlueGenerator::replaceStackPointerGlobal() {
   wasm.removeGlobal(stackPointer->name);
 }
 
-struct JSCallWalker : public PostWalker<JSCallWalker> {
-  Module &wasm;
-  JSCallWalker(Module &_wasm) : wasm(_wasm) {
-    if (wasm.table.segments.size() == 0) {
-      auto emptySegment =
-          wasm.allocator.alloc<Const>()->set(Literal(uint32_t(0)));
-      wasm.table.segments.emplace_back(emptySegment);
-    }
-    const auto& tableSegmentData = wasm.table.segments[0].data;
-
-    jsCallStartIndex =
-        wasm.table.segments[0].offset->cast<Const>()->value.getInteger();
-    // Check if jsCalls have already been created
-    for (Index i = 0; i < tableSegmentData.size(); ++i) {
-      if (tableSegmentData[i].startsWith("jsCall_")) {
-        jsCallStartIndex += i;
-        return;
-      }
-    }
-    jsCallStartIndex += tableSegmentData.size();
-  }
-
-  // Gather all function signatures used in call_indirect, because any of them
-  // can be used to call function pointers created by emscripten's addFunction.
-  void visitCallIndirect(CallIndirect *curr) {
-    // dynCall thunks are generated in binaryen and call_indirect instructions
-    // within them cannot be used to call function pointers returned by
-    // emscripten's addFunction.
-    if (!getFunction()->name.startsWith("dynCall_")) {
-      indirectlyCallableSigs.insert(
-          getSig(wasm.getFunctionType(curr->fullType)));
-    }
-  }
-
-  bool createJSCallThunks;
-  Index jsCallStartIndex;
-  // Function type signatures used in call_indirect instructions
-  std::set<std::string> indirectlyCallableSigs;
-};
-
-JSCallWalker getJSCallWalker(Module& wasm) {
-  JSCallWalker walker(wasm);
-  walker.walkModule(&wasm);
-  return walker;
-}
-
-void EmscriptenGlueGenerator::generateJSCallThunks(
-    unsigned numReservedFunctionPointers) {
-  if (numReservedFunctionPointers == 0)
-    return;
-
-  JSCallWalker walker = getJSCallWalker(wasm);
-  auto& tableSegmentData = wasm.table.segments[0].data;
-  unsigned numEntriesAdded = 0;
-  for (std::string sig : walker.indirectlyCallableSigs) {
-    // Add imports for jsCall_sig (e.g. jsCall_vi).
-    // Imported jsCall_sig functions have their first parameter as an index to
-    // the function table, so we should prepend an 'i' to parameters' signature
-    // (e.g. If the signature of the callee is 'vi', the imported jsCall_vi
-    // function would have signature 'vii'.)
-    std::string importSig = std::string(1, sig[0]) + 'i' + sig.substr(1);
-    FunctionType *importType = ensureFunctionType(importSig, &wasm);
-    auto import = new Function;
-    import->name = import->base = "jsCall_" + sig;
-    import->module = ENV;
-    import->type = importType->name;
-    FunctionTypeUtils::fillFunction(import, importType);
-    wasm.addFunction(import);
-    FunctionType *funcType = ensureFunctionType(sig, &wasm);
-
-    // Create jsCall_sig_index thunks (e.g. jsCall_vi_0, jsCall_vi_1, ...)
-    // e.g. If # of reserved function pointers (given by a command line
-    // argument) is 3 and there are two possible signature 'vi' and 'ii', the
-    // genereated thunks will be jsCall_vi_0, jsCall_vi_1, jsCall_vi_2,
-    // jsCall_ii_0, jsCall_ii_1, and jsCall_ii_2.
-    for (unsigned fp = 0; fp < numReservedFunctionPointers; ++fp) {
-      std::vector<NameType> params;
-      int p = 0;
-      for (const auto& ty : funcType->params) {
-        params.emplace_back(std::to_string(p++), ty);
-      }
-      Function* f = builder.makeFunction(
-          std::string("jsCall_") + sig + "_" + std::to_string(fp),
-          std::move(params), funcType->result, {});
-      std::vector<Expression*> args;
-      args.push_back(builder.makeConst(Literal(fp)));
-      for (unsigned i = 0; i < funcType->params.size(); ++i) {
-        args.push_back(builder.makeGetLocal(i, funcType->params[i]));
-      }
-      Expression* call =
-          builder.makeCall(import->name, args, funcType->result);
-      f->body = call;
-      wasm.addFunction(f);
-      tableSegmentData.push_back(f->name);
-      numEntriesAdded++;
-    }
-  }
-  wasm.table.initial.addr += numEntriesAdded;
-  if (wasm.table.max != Table::kUnlimitedSize) {
-    wasm.table.max.addr += numEntriesAdded;
-  }
-}
-
 std::vector<Address> getSegmentOffsets(Module& wasm) {
   std::vector<Address> segmentOffsets;
   for (unsigned i = 0; i < wasm.memory.segments.size(); ++i) {
@@ -780,8 +677,7 @@ void printSet(std::ostream& o, C& c) {
 }
 
 std::string EmscriptenGlueGenerator::generateEmscriptenMetadata(
-    Address staticBump, std::vector<Name> const& initializerFunctions,
-    unsigned numReservedFunctionPointers) {
+    Address staticBump, std::vector<Name> const& initializerFunctions) {
   bool commaFirst;
   auto nextElement = [&commaFirst]() {
     if (commaFirst) {
@@ -844,18 +740,6 @@ std::string EmscriptenGlueGenerator::generateEmscriptenMetadata(
     meta << "\n  ],\n";
   }
 
-  if (numReservedFunctionPointers) {
-    JSCallWalker jsCallWalker = getJSCallWalker(wasm);
-    meta << "  \"jsCallStartIndex\": " << jsCallWalker.jsCallStartIndex << ",\n";
-    meta << "  \"jsCallFuncType\": [";
-    commaFirst = true;
-    for (std::string sig : jsCallWalker.indirectlyCallableSigs) {
-      meta << nextElement();
-      meta << "\"" << sig << "\"";
-    }
-    meta << "\n  ],\n";
-  }
-
   // Avoid adding duplicate imports to `declares' or `invokeFuncs`.  Even
   // though we might import the same function multiple times (i.e. with
   // different sigs) we only need to list is in the metadata once.
@@ -870,8 +754,7 @@ std::string EmscriptenGlueGenerator::generateEmscriptenMetadata(
   ModuleUtils::iterImportedFunctions(wasm, [&](Function* import) {
     if (emJsWalker.codeByName.count(import->base.str) == 0 &&
         !import->base.startsWith(EMSCRIPTEN_ASM_CONST.str) &&
-        !import->base.startsWith("invoke_") &&
-        !import->base.startsWith("jsCall_")) {
+        !import->base.startsWith("invoke_")) {
       if (declares.insert(import->base.str).second) {
         meta << nextElement() << '"' << import->base.str << '"';
       }
diff --git a/test/lld/reserved_func_ptr.wast.jscall.out b/test/lld/reserved_func_ptr.wast.jscall.out
deleted file mode 100644
index e176ba231..000000000
--- a/test/lld/reserved_func_ptr.wast.jscall.out
+++ /dev/null
@@ -1,359 +0,0 @@
-(module
- (type $0 (func))
- (type $1 (func (param i32)))
- (type $2 (func (param i32 i32) (result i32)))
- (type $3 (func (param f32 f32 i32) (result f32)))
- (type $4 (func (param f64 i32) (result f64)))
- (type $5 (func (param i32 i32 i32)))
- (type $6 (func (param i32) (result i32)))
- (type $FUNCSIG$ii (func (param i32) (result i32)))
- (type $FUNCSIG$viii (func (param i32 i32 i32)))
- (type $FUNCSIG$didi (func (param i32 f64 i32) (result f64)))
- (type $FUNCSIG$ddi (func (param f64 i32) (result f64)))
- (type $FUNCSIG$fiffi (func (param i32 f32 f32 i32) (result f32)))
- (type $FUNCSIG$fffi (func (param f32 f32 i32) (result f32)))
- (type $FUNCSIG$iiii (func (param i32 i32 i32) (result i32)))
- (type $FUNCSIG$iii (func (param i32 i32) (result i32)))
- (type $FUNCSIG$vi (func (param i32)))
- (type $FUNCSIG$v (func))
- (type $FUNCSIG$vii (func (param i32 i32)))
- (type $FUNCSIG$viiii (func (param i32 i32 i32 i32)))
- (type $legaltype$jsCall_fffi (func (param i32 f64 f64 i32) (result f64)))
- (import "env" "_Z4atoiPKc" (func $_Z4atoiPKc (param i32) (result i32)))
- (import "env" "jsCall_ddi" (func $jsCall_ddi (param i32 f64 i32) (result f64)))
- (import "env" "jsCall_iii" (func $jsCall_iii (param i32 i32 i32) (result i32)))
- (import "env" "jsCall_v" (func $jsCall_v (param i32)))
- (import "env" "jsCall_vi" (func $jsCall_vi (param i32 i32)))
- (import "env" "jsCall_viii" (func $jsCall_viii (param i32 i32 i32 i32)))
- (import "env" "jsCall_fffi" (func $legalimport$jsCall_fffi (param i32 f64 f64 i32) (result f64)))
- (memory $0 2)
- (table $0 21 21 funcref)
- (elem (i32.const 1) $_Z18address_taken_funciii $_Z19address_taken_func2iii $jsCall_ddi_0 $jsCall_ddi_1 $jsCall_ddi_2 $jsCall_fffi_0 $jsCall_fffi_1 $jsCall_fffi_2 $jsCall_iii_0 $jsCall_iii_1 $jsCall_iii_2 $jsCall_v_0 $jsCall_v_1 $jsCall_v_2 $jsCall_vi_0 $jsCall_vi_1 $jsCall_vi_2 $jsCall_viii_0 $jsCall_viii_1 $jsCall_viii_2)
- (global $global$0 (mut i32) (i32.const 16384))
- (global $global$1 i32 (i32.const 66112))
- (global $global$2 i32 (i32.const 568))
- (export "memory" (memory $0))
- (export "__post_instantiate" (func $__wasm_call_ctors))
- (export "main" (func $main))
- (export "__heap_base" (global $global$1))
- (export "__data_end" (global $global$2))
- (export "stackSave" (func $stackSave))
- (export "stackAlloc" (func $stackAlloc))
- (export "stackRestore" (func $stackRestore))
- (export "__growWasmMemory" (func $__growWasmMemory))
- (export "dynCall_viii" (func $dynCall_viii))
- (func $_Z18address_taken_funciii (; 7 ;) (type $5) (param $var$0 i32) (param $var$1 i32) (param $var$2 i32)
-  (nop)
- )
- (func $_Z19address_taken_func2iii (; 8 ;) (type $5) (param $var$0 i32) (param $var$1 i32) (param $var$2 i32)
-  (nop)
- )
- (func $main (; 9 ;) (type $2) (param $var$0 i32) (param $var$1 i32) (result i32)
-  (local $var$2 i32)
-  (local $var$3 i32)
-  (local $var$4 i32)
-  (local $var$5 i32)
-  (local.set $var$2
-   (call $_Z4atoiPKc
-    (i32.load offset=4
-     (local.get $var$1)
-    )
-   )
-  )
-  (local.set $var$3
-   (call $_Z4atoiPKc
-    (i32.load offset=8
-     (local.get $var$1)
-    )
-   )
-  )
-  (local.set $var$4
-   (call $_Z4atoiPKc
-    (i32.load offset=12
-     (local.get $var$1)
-    )
-   )
-  )
-  (local.set $var$5
-   (call $_Z4atoiPKc
-    (i32.load offset=16
-     (local.get $var$1)
-    )
-   )
-  )
-  (local.set $var$1
-   (call $_Z4atoiPKc
-    (i32.load offset=20
-     (local.get $var$1)
-    )
-   )
-  )
-  (call_indirect (type $0)
-   (local.get $var$2)
-  )
-  (call_indirect (type $1)
-   (i32.const 3)
-   (local.get $var$3)
-  )
-  (drop
-   (call_indirect (type $2)
-    (i32.const 4)
-    (i32.const 5)
-    (local.get $var$4)
-   )
-  )
-  (drop
-   (call_indirect (type $3)
-    (f32.const 3.0999999046325684)
-    (f32.const 4.199999809265137)
-    (i32.const 5)
-    (local.get $var$5)
-   )
-  )
-  (drop
-   (call_indirect (type $4)
-    (f64.const 4.2)
-    (i32.const 5)
-    (local.get $var$1)
-   )
-  )
-  (call_indirect (type $5)
-   (i32.const 1)
-   (i32.const 2)
-   (i32.const 3)
-   (select
-    (i32.const 1)
-    (i32.const 2)
-    (i32.gt_s
-     (local.get $var$0)
-     (i32.const 3)
-    )
-   )
-  )
-  (i32.const 0)
- )
- (func $__wasm_call_ctors (; 10 ;) (type $0)
-  (nop)
- )
- (func $stackSave (; 11 ;) (result i32)
-  (global.get $global$0)
- )
- (func $stackAlloc (; 12 ;) (param $0 i32) (result i32)
-  (local $1 i32)
-  (global.set $global$0
-   (local.tee $1
-    (i32.and
-     (i32.sub
-      (global.get $global$0)
-      (local.get $0)
-     )
-     (i32.const -16)
-    )
-   )
-  )
-  (local.get $1)
- )
- (func $stackRestore (; 13 ;) (param $0 i32)
-  (global.set $global$0
-   (local.get $0)
-  )
- )
- (func $__growWasmMemory (; 14 ;) (param $newSize i32) (result i32)
-  (grow_memory
-   (local.get $newSize)
-  )
- )
- (func $dynCall_viii (; 15 ;) (param $fptr i32) (param $0 i32) (param $1 i32) (param $2 i32)
-  (call_indirect (type $FUNCSIG$viii)
-   (local.get $0)
-   (local.get $1)
-   (local.get $2)
-   (local.get $fptr)
-  )
- )
- (func $jsCall_ddi_0 (; 16 ;) (param $0 f64) (param $1 i32) (result f64)
-  (call $jsCall_ddi
-   (i32.const 0)
-   (local.get $0)
-   (local.get $1)
-  )
- )
- (func $jsCall_ddi_1 (; 17 ;) (param $0 f64) (param $1 i32) (result f64)
-  (call $jsCall_ddi
-   (i32.const 1)
-   (local.get $0)
-   (local.get $1)
-  )
- )
- (func $jsCall_ddi_2 (; 18 ;) (param $0 f64) (param $1 i32) (result f64)
-  (call $jsCall_ddi
-   (i32.const 2)
-   (local.get $0)
-   (local.get $1)
-  )
- )
- (func $jsCall_fffi_0 (; 19 ;) (param $0 f32) (param $1 f32) (param $2 i32) (result f32)
-  (call $legalfunc$jsCall_fffi
-   (i32.const 0)
-   (local.get $0)
-   (local.get $1)
-   (local.get $2)
-  )
- )
- (func $jsCall_fffi_1 (; 20 ;) (param $0 f32) (param $1 f32) (param $2 i32) (result f32)
-  (call $legalfunc$jsCall_fffi
-   (i32.const 1)
-   (local.get $0)
-   (local.get $1)
-   (local.get $2)
-  )
- )
- (func $jsCall_fffi_2 (; 21 ;) (param $0 f32) (param $1 f32) (param $2 i32) (result f32)
-  (call $legalfunc$jsCall_fffi
-   (i32.const 2)
-   (local.get $0)
-   (local.get $1)
-   (local.get $2)
-  )
- )
- (func $jsCall_iii_0 (; 22 ;) (param $0 i32) (param $1 i32) (result i32)
-  (call $jsCall_iii
-   (i32.const 0)
-   (local.get $0)
-   (local.get $1)
-  )
- )
- (func $jsCall_iii_1 (; 23 ;) (param $0 i32) (param $1 i32) (result i32)
-  (call $jsCall_iii
-   (i32.const 1)
-   (local.get $0)
-   (local.get $1)
-  )
- )
- (func $jsCall_iii_2 (; 24 ;) (param $0 i32) (param $1 i32) (result i32)
-  (call $jsCall_iii
-   (i32.const 2)
-   (local.get $0)
-   (local.get $1)
-  )
- )
- (func $jsCall_v_0 (; 25 ;)
-  (call $jsCall_v
-   (i32.const 0)
-  )
- )
- (func $jsCall_v_1 (; 26 ;)
-  (call $jsCall_v
-   (i32.const 1)
-  )
- )
- (func $jsCall_v_2 (; 27 ;)
-  (call $jsCall_v
-   (i32.const 2)
-  )
- )
- (func $jsCall_vi_0 (; 28 ;) (param $0 i32)
-  (call $jsCall_vi
-   (i32.const 0)
-   (local.get $0)
-  )
- )
- (func $jsCall_vi_1 (; 29 ;) (param $0 i32)
-  (call $jsCall_vi
-   (i32.const 1)
-   (local.get $0)
-  )
- )
- (func $jsCall_vi_2 (; 30 ;) (param $0 i32)
-  (call $jsCall_vi
-   (i32.const 2)
-   (local.get $0)
-  )
- )
- (func $jsCall_viii_0 (; 31 ;) (param $0 i32) (param $1 i32) (param $2 i32)
-  (call $jsCall_viii
-   (i32.const 0)
-   (local.get $0)
-   (local.get $1)
-   (local.get $2)
-  )
- )
- (func $jsCall_viii_1 (; 32 ;) (param $0 i32) (param $1 i32) (param $2 i32)
-  (call $jsCall_viii
-   (i32.const 1)
-   (local.get $0)
-   (local.get $1)
-   (local.get $2)
-  )
- )
- (func $jsCall_viii_2 (; 33 ;) (param $0 i32) (param $1 i32) (param $2 i32)
-  (call $jsCall_viii
-   (i32.const 2)
-   (local.get $0)
-   (local.get $1)
-   (local.get $2)
-  )
- )
- (func $legalfunc$jsCall_fffi (; 34 ;) (param $0 i32) (param $1 f32) (param $2 f32) (param $3 i32) (result f32)
-  (f32.demote_f64
-   (call $legalimport$jsCall_fffi
-    (local.get $0)
-    (f64.promote_f32
-     (local.get $1)
-    )
-    (f64.promote_f32
-     (local.get $2)
-    )
-    (local.get $3)
-   )
-  )
- )
-)
-(;
---BEGIN METADATA --
-{
-  "staticBump": 0,
-  "tableSize": 21,
-  "initializers": [
-    "__post_instantiate"
-  ],
-  "jsCallStartIndex": 3,
-  "jsCallFuncType": [
-    "ddi",
-    "fffi",
-    "iii",
-    "v",
-    "vi",
-    "viii"
-  ],
-  "declares": [
-    "_Z4atoiPKc"
-  ],
-  "externs": [
-  ],
-  "implementedFunctions": [
-    "___post_instantiate",
-    "_main",
-    "_stackSave",
-    "_stackAlloc",
-    "_stackRestore",
-    "___growWasmMemory",
-    "_dynCall_viii"
-  ],
-  "exports": [
-    "memory",
-    "__post_instantiate",
-    "main",
-    "__heap_base",
-    "__data_end",
-    "stackSave",
-    "stackAlloc",
-    "stackRestore",
-    "__growWasmMemory",
-    "dynCall_viii"
-  ],
-  "invokeFuncs": [
-  ]
-}
--- END METADATA --
-;)