legalize invokes even when doing minimal legalization, as js needs them (#1883)

See [emscripten-core/emscripten#7679 

3 files changed, 71 insertions, 43 deletions

diff --git a/src/passes/LegalizeJSInterface.cpp b/src/passes/LegalizeJSInterface.cpp
index 8a68ccc88..cef4e38ea 100644
--- a/src/passes/LegalizeJSInterface.cpp
+++ b/src/passes/LegalizeJSInterface.cpp
@@ -63,58 +63,57 @@ struct LegalizeJSInterface : public Pass {
         }
       }
     }
-    if (full) {
-      // Avoid iterator invalidation later.
-      std::vector<Function*> originalFunctions;
-      for (auto& func : module->functions) {
-        originalFunctions.push_back(func.get());
-      }
-      // for each illegal import, we must call a legalized stub instead
-      for (auto* im : originalFunctions) {
-        if (im->imported() && isIllegal(module->getFunctionType(im->type))) {
-          auto funcName = makeLegalStubForCalledImport(im, module);
-          illegalImportsToLegal[im->name] = funcName;
-          // we need to use the legalized version in the table, as the import from JS
-          // is legal for JS. Our stub makes it look like a native wasm function.
-          for (auto& segment : module->table.segments) {
-            for (auto& name : segment.data) {
-              if (name == im->name) {
-                name = funcName;
-              }
+    // Avoid iterator invalidation later.
+    std::vector<Function*> originalFunctions;
+    for (auto& func : module->functions) {
+      originalFunctions.push_back(func.get());
+    }
+    // for each illegal import, we must call a legalized stub instead
+    for (auto* im : originalFunctions) {
+      if (im->imported() && isIllegal(module->getFunctionType(im->type))
+                         && shouldBeLegalized(im)) {
+        auto funcName = makeLegalStubForCalledImport(im, module);
+        illegalImportsToLegal[im->name] = funcName;
+        // we need to use the legalized version in the table, as the import from JS
+        // is legal for JS. Our stub makes it look like a native wasm function.
+        for (auto& segment : module->table.segments) {
+          for (auto& name : segment.data) {
+            if (name == im->name) {
+              name = funcName;
             }
           }
         }
       }
-      if (illegalImportsToLegal.size() > 0) {
-        for (auto& pair : illegalImportsToLegal) {
-          module->removeFunction(pair.first);
-        }
+    }
+    if (illegalImportsToLegal.size() > 0) {
+      for (auto& pair : illegalImportsToLegal) {
+        module->removeFunction(pair.first);
+      }
 
-        // fix up imports: call_import of an illegal must be turned to a call of a legal
+      // fix up imports: call_import of an illegal must be turned to a call of a legal
 
-        struct FixImports : public WalkerPass<PostWalker<FixImports>> {
-          bool isFunctionParallel() override { return true; }
+      struct FixImports : public WalkerPass<PostWalker<FixImports>> {
+        bool isFunctionParallel() override { return true; }
 
-          Pass* create() override { return new FixImports(illegalImportsToLegal); }
+        Pass* create() override { return new FixImports(illegalImportsToLegal); }
 
-          std::map<Name, Name>* illegalImportsToLegal;
+        std::map<Name, Name>* illegalImportsToLegal;
 
-          FixImports(std::map<Name, Name>* illegalImportsToLegal) : illegalImportsToLegal(illegalImportsToLegal) {}
+        FixImports(std::map<Name, Name>* illegalImportsToLegal) : illegalImportsToLegal(illegalImportsToLegal) {}
 
-          void visitCall(Call* curr) {
-            auto iter = illegalImportsToLegal->find(curr->target);
-            if (iter == illegalImportsToLegal->end()) return;
+        void visitCall(Call* curr) {
+          auto iter = illegalImportsToLegal->find(curr->target);
+          if (iter == illegalImportsToLegal->end()) return;
 
-            if (iter->second == getFunction()->name) return; // inside the stub function itself, is the one safe place to do the call
-            replaceCurrent(Builder(*getModule()).makeCall(iter->second, curr->operands, curr->type));
-          }
-        };
+          if (iter->second == getFunction()->name) return; // inside the stub function itself, is the one safe place to do the call
+          replaceCurrent(Builder(*getModule()).makeCall(iter->second, curr->operands, curr->type));
+        }
+      };
 
-        PassRunner passRunner(module);
-        passRunner.setIsNested(true);
-        passRunner.add<FixImports>(&illegalImportsToLegal);
-        passRunner.run();
-      }
+      PassRunner passRunner(module);
+      passRunner.setIsNested(true);
+      passRunner.add<FixImports>(&illegalImportsToLegal);
+      passRunner.run();
     }
   }
 
@@ -131,12 +130,20 @@ private:
     return false;
   }
 
+  // Check if an export should be legalized.
   bool shouldBeLegalized(Export* ex, Function* func) {
     if (full) return true;
     // We are doing minimal legalization - just what JS needs.
     return ex->name.startsWith("dynCall_");
   }
 
+  // Check if an import should be legalized.
+  bool shouldBeLegalized(Function* im) {
+    if (full) return true;
+    // We are doing minimal legalization - just what JS needs.
+    return im->module == ENV && im->base.startsWith("invoke_");
+  }
+
   // JS calls the export, so it must call a legal stub that calls the actual wasm function
   Name makeLegalStub(Function* func, Module* module) {
     Builder builder(*module);
diff --git a/test/passes/legalize-js-interface-minimally.txt b/test/passes/legalize-js-interface-minimally.txt
index c9fab0fc0..2841ddb09 100644
--- a/test/passes/legalize-js-interface-minimally.txt
+++ b/test/passes/legalize-js-interface-minimally.txt
@@ -1,23 +1,29 @@
 (module
  (type $FUNCSIG$j (func (result i64)))
+ (type $FUNCSIG$vj (func (param i64)))
  (type $FUNCSIG$vi (func (param i32)))
+ (type $legaltype$invoke_vj (func (param i32 i32)))
  (import "env" "imported" (func $imported (result i64)))
  (import "env" "setTempRet0" (func $setTempRet0 (param i32)))
+ (import "env" "invoke_vj" (func $legalimport$invoke_vj (param i32 i32)))
  (export "func" (func $func))
  (export "dynCall_foo" (func $legalstub$dyn))
- (func $func (; 2 ;) (type $FUNCSIG$j) (result i64)
+ (func $func (; 3 ;) (type $FUNCSIG$j) (result i64)
   (drop
    (call $imported)
   )
+  (call $legalfunc$invoke_vj
+   (i64.const 0)
+  )
   (unreachable)
  )
- (func $dyn (; 3 ;) (type $FUNCSIG$j) (result i64)
+ (func $dyn (; 4 ;) (type $FUNCSIG$j) (result i64)
   (drop
    (call $imported)
   )
   (unreachable)
  )
- (func $legalstub$dyn (; 4 ;) (result i32)
+ (func $legalstub$dyn (; 5 ;) (result i32)
   (local $0 i64)
   (local.set $0
    (call $dyn)
@@ -34,6 +40,19 @@
    (local.get $0)
   )
  )
+ (func $legalfunc$invoke_vj (; 6 ;) (param $0 i64)
+  (call $legalimport$invoke_vj
+   (i32.wrap_i64
+    (local.get $0)
+   )
+   (i32.wrap_i64
+    (i64.shr_u
+     (local.get $0)
+     (i64.const 32)
+    )
+   )
+  )
+ )
 )
 (module
 )
diff --git a/test/passes/legalize-js-interface-minimally.wast b/test/passes/legalize-js-interface-minimally.wast
index 2e003a521..e820734be 100644
--- a/test/passes/legalize-js-interface-minimally.wast
+++ b/test/passes/legalize-js-interface-minimally.wast
@@ -1,9 +1,11 @@
 (module
   (import "env" "imported" (func $imported (result i64)))
+  (import "env" "invoke_vj" (func $invoke_vj (param i64)))
   (export "func" (func $func))
   (export "dynCall_foo" (func $dyn))
   (func $func (result i64)
     (drop (call $imported))
+    (call $invoke_vj (i64.const 0))
     (unreachable)
   )
   (func $dyn (result i64)