Wasm2js memory fixes (#2003)

* I64ToI32Lowering - don't assume address 0 is a hardcoded location for scratch memory. Import __tempMemory__ for that.
* RemoveNonJSOps - also use __tempMemory__. Oddly here the address was a hardcoded 1024 (perhaps where the rust program put a static global?).
* Support imported ints in wasm2js, coercing them as needed.
* Add "env" import support in the tests, since now we emit imports from there.
* Make wasm2js tests split out multi-module tests using split_wast which is more robust and avoids emitting multiple outputs in one file (which makes no sense for ES6 modules)

4 files changed, 92 insertions, 47 deletions

diff --git a/src/passes/I64ToI32Lowering.cpp b/src/passes/I64ToI32Lowering.cpp
index ae69ec19d..208aac9ea 100644
--- a/src/passes/I64ToI32Lowering.cpp
+++ b/src/passes/I64ToI32Lowering.cpp
@@ -600,16 +600,17 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
     // our f64 through memory at address 0
     TempVar highBits = getTemp();
     Block *result = builder->blockify(
-      builder->makeStore(8, 0, 8, builder->makeConst(Literal(int32_t(0))), curr->value, f64),
+      builder->makeStore(8, 0, 8, makeGetTempMemory(), curr->value, f64),
       builder->makeSetLocal(
         highBits,
-        builder->makeLoad(4, true, 4, 4, builder->makeConst(Literal(int32_t(0))), i32)
+        builder->makeLoad(4, true, 4, 4, makeGetTempMemory(), i32)
       ),
-      builder->makeLoad(4, true, 0, 4, builder->makeConst(Literal(int32_t(0))), i32)
+      builder->makeLoad(4, true, 0, 4, makeGetTempMemory(), i32)
     );
     setOutParam(result, std::move(highBits));
     replaceCurrent(result);
-    ensureMinimalMemory();
+    MemoryUtils::ensureExists(getModule()->memory);
+    ensureTempMemoryGlobal();
   }
 
   void lowerReinterpretInt64(Unary* curr) {
@@ -617,18 +618,33 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
     // our i64 through memory at address 0
     TempVar highBits = fetchOutParam(curr->value);
     Block *result = builder->blockify(
-      builder->makeStore(4, 0, 4, builder->makeConst(Literal(int32_t(0))), curr->value, i32),
-      builder->makeStore(4, 4, 4, builder->makeConst(Literal(int32_t(0))), builder->makeGetLocal(highBits, i32), i32),
-      builder->makeLoad(8, true, 0, 8, builder->makeConst(Literal(int32_t(0))), f64)
+      builder->makeStore(4, 0, 4, makeGetTempMemory(), curr->value, i32),
+      builder->makeStore(4, 4, 4, makeGetTempMemory(), builder->makeGetLocal(highBits, i32), i32),
+      builder->makeLoad(8, true, 0, 8, makeGetTempMemory(), f64)
     );
     replaceCurrent(result);
-    ensureMinimalMemory();
+    MemoryUtils::ensureExists(getModule()->memory);
+    ensureTempMemoryGlobal();
   }
 
-  // Ensure memory exists with a minimal size, enough for round-tripping operations on
-  // address 0, which we need for reinterpret operations.
-  void ensureMinimalMemory() {
-    MemoryUtils::ensureExists(getModule()->memory);
+  Name tempMemory = "__tempMemory__";
+
+  void ensureTempMemoryGlobal() {
+    // Ensure the existence of an imported global, __tempMemory__, which points to 8
+    // bytes of scratch memory we can use for roundtrip purposes.
+    if (!getModule()->getGlobalOrNull(tempMemory)) {
+      auto global = make_unique<Global>();
+      global->name = tempMemory;
+      global->type = i32;
+      global->mutable_ = false;
+      global->module = ENV;
+      global->base = tempMemory;
+      getModule()->addGlobal(global.release());
+    }
+  }
+
+  Expression* makeGetTempMemory() {
+    return builder->makeGetGlobal(tempMemory, i32);
   }
 
   void lowerTruncFloatToInt(Unary *curr) {
diff --git a/src/passes/RemoveNonJSOps.cpp b/src/passes/RemoveNonJSOps.cpp
index 342f59eba..a98189584 100644
--- a/src/passes/RemoveNonJSOps.cpp
+++ b/src/passes/RemoveNonJSOps.cpp
@@ -43,6 +43,7 @@ namespace wasm {
 struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
   std::unique_ptr<Builder> builder;
   std::unordered_set<Name> neededIntrinsics;
+  std::set<std::pair<Name, Type>> neededImportedGlobals;
 
   bool isFunctionParallel() override { return false; }
 
@@ -101,6 +102,21 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
 
     // Intrinsics may use memory, so ensure the module has one.
     MemoryUtils::ensureExists(module->memory);
+
+    // Add missing globals
+    for (auto& pair : neededImportedGlobals) {
+      auto name = pair.first;
+      auto type = pair.second;
+      if (!getModule()->getGlobalOrNull(name)) {
+        auto global = make_unique<Global>();
+        global->name = name;
+        global->type = type;
+        global->mutable_ = false;
+        global->module = ENV;
+        global->base = name;
+        module->addGlobal(global.release());
+      }
+    }
   }
 
   void addNeededFunctions(Module &m, Name name, std::set<Name> &needed) {
@@ -121,7 +137,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
     PostWalker<RemoveNonJSOpsPass>::doWalkFunction(func);
   }
 
-  void visitLoad(Load *curr) {
+  void visitLoad(Load* curr) {
     if (curr->align == 0 || curr->align >= curr->bytes) {
       return;
     }
@@ -143,7 +159,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
     }
   }
 
-  void visitStore(Store *curr) {
+  void visitStore(Store* curr) {
     if (curr->align == 0 || curr->align >= curr->bytes) {
       return;
     }
@@ -165,7 +181,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
     }
   }
 
-  void visitBinary(Binary *curr) {
+  void visitBinary(Binary* curr) {
     Name name;
     switch (curr->op) {
       case CopySignFloat32:
@@ -207,7 +223,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
     replaceCurrent(builder->makeCall(name, {curr->left, curr->right}, curr->type));
   }
 
-  void rewriteCopysign(Binary *curr) {
+  void rewriteCopysign(Binary* curr) {
     Literal signBit, otherBits;
     UnaryOp int2float, float2int;
     BinaryOp bitAnd, bitOr;
@@ -259,7 +275,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
     );
   }
 
-  void visitUnary(Unary *curr) {
+  void visitUnary(Unary* curr) {
     Name functionCall;
     switch (curr->op) {
       case NearestFloat32:
@@ -295,9 +311,13 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
     neededIntrinsics.insert(functionCall);
     replaceCurrent(builder->makeCall(functionCall, {curr->value}, curr->type));
   }
+
+  void visitGetGlobal(GetGlobal* curr) {
+    neededImportedGlobals.insert(std::make_pair(curr->name, curr->type));
+  }
 };
 
-Pass *createRemoveNonJSOpsPass() {
+Pass* createRemoveNonJSOpsPass() {
   return new RemoveNonJSOpsPass();
 }
 
diff --git a/src/passes/wasm-intrinsics.wast b/src/passes/wasm-intrinsics.wast
index 26687508d..a8ee5be0b 100644
--- a/src/passes/wasm-intrinsics.wast
+++ b/src/passes/wasm-intrinsics.wast
@@ -5,6 +5,13 @@
 ;; these pretty early so they can continue to be optimized by further passes
 ;; (aka inlining and whatnot)
 ;;
+;; LOCAL MODS done by hand afterwards:
+;;  * Remove hardcoded address 1024 (apparently a free memory location rustc
+;;    thinks is ok to use?); add a global __tempMemory__ which is used for that
+;;    purpose.
+;;  * Fix function type of __wasm_ctz_i64, which was wrong somehow,
+;;    i32, i32 => i32 instead of i64 => i64
+;;
 ;; [1]: https://gist.github.com/alexcrichton/e7ea67bcdd17ce4b6254e66f77165690
 
 (module
@@ -13,7 +20,9 @@
  (type $2 (func (param f64) (result f64)))
  (type $3 (func (param i32) (result i32)))
  (type $4 (func (param i32 i32) (result i32)))
+ (type $5 (func (param i64) (result i64)))
  (import "env" "memory" (memory $0 17))
+ (import "env" "__tempMemory__" (global $__tempMemory__ i32))
  (export "__wasm_i64_sdiv" (func $__wasm_i64_sdiv))
  (export "__wasm_i64_udiv" (func $__wasm_i64_udiv))
  (export "__wasm_i64_srem" (func $__wasm_i64_srem))
@@ -128,7 +137,7 @@
    )
   )
   (i64.load
-   (i32.const 1024)
+   (global.get $__tempMemory__)
   )
  )
  ;; lowering of the i64.mul instruction, return $var0 * $var$1
@@ -192,7 +201,7 @@
   (i32.const 32)
  )
  ;; lowering of the i64.ctz instruction, counting the number of zeros in $var$0
- (func $__wasm_ctz_i64 (; 8 ;) (type $4) (param $var$0 i64) (result i64)
+ (func $__wasm_ctz_i64 (; 8 ;) (type $5) (param $var$0 i64) (result i64)
   (if
    (i32.eqz
     (i64.eqz
@@ -571,7 +580,7 @@
               )
              )
              (i64.store
-              (i32.const 1024)
+              (global.get $__tempMemory__)
               (i64.extend_i32_u
                (i32.sub
                 (local.tee $var$2
@@ -633,7 +642,7 @@
              )
             )
             (i64.store
-             (i32.const 1024)
+             (global.get $__tempMemory__)
              (i64.or
               (i64.shl
                (i64.extend_i32_u
@@ -714,7 +723,7 @@
          (br $label$3)
         )
         (i64.store
-         (i32.const 1024)
+         (global.get $__tempMemory__)
          (i64.shl
           (i64.extend_i32_u
            (i32.sub
@@ -757,7 +766,7 @@
        (br $label$2)
       )
       (i64.store
-       (i32.const 1024)
+       (global.get $__tempMemory__)
        (i64.extend_i32_u
         (i32.and
          (local.get $var$4)
@@ -889,7 +898,7 @@
      )
     )
     (i64.store
-     (i32.const 1024)
+     (global.get $__tempMemory__)
      (local.get $var$5)
     )
     (return
@@ -903,7 +912,7 @@
     )
    )
    (i64.store
-    (i32.const 1024)
+    (global.get $__tempMemory__)
     (local.get $var$0)
    )
    (local.set $var$0
diff --git a/src/wasm2js.h b/src/wasm2js.h
index 19c12a501..fab39267d 100644
--- a/src/wasm2js.h
+++ b/src/wasm2js.h
@@ -486,12 +486,16 @@ void Wasm2JSBuilder::addGlobalImport(Ref ast, Global* import) {
   Ref theVar = ValueBuilder::makeVar();
   ast->push_back(theVar);
   Ref module = ValueBuilder::makeName(ENV); // TODO: handle nested module imports
+  Ref value = ValueBuilder::makeDot(
+    module,
+    fromName(import->base, NameScope::Top)
+  );
+  if (import->type == i32) {
+    value = makeAsmCoercion(value, ASM_INT);
+  }
   ValueBuilder::appendToVar(theVar,
     fromName(import->name, NameScope::Top),
-    ValueBuilder::makeDot(
-      module,
-      fromName(import->base, NameScope::Top)
-    )
+    value
   );
 }
 
@@ -2053,7 +2057,6 @@ void Wasm2JSGlue::emitPreES6() {
   ImportInfo imports(wasm);
 
   ModuleUtils::iterImportedGlobals(wasm, [&](Global* import) {
-    Fatal() << "non-function imports aren't supported yet\n";
     noteImport(import->module, import->base);
   });
   ModuleUtils::iterImportedFunctions(wasm, [&](Function* import) {
@@ -2074,23 +2077,20 @@ void Wasm2JSGlue::emitPost() {
 void Wasm2JSGlue::emitPostEmscripten() {
   emitMemory("wasmMemory.buffer", "writeSegment");
 
-  out << "return asmFunc(\n"
-      << "  {\n"
-      << "    'env': asmLibraryArg,\n"
-      << "    'global': {\n"
-      << "      'Int8Array': Int8Array,\n"
-      << "      'Int16Array': Int16Array,\n"
-      << "      'Int32Array': Int32Array,\n"
-      << "      'Uint8Array': Uint8Array,\n"
-      << "      'Uint16Array': Uint16Array,\n"
-      << "      'Uint32Array': Uint32Array,\n"
-      << "      'Float32Array': Float32Array,\n"
-      << "      'Float64Array': Float64Array,\n"
-      << "      'NaN': NaN,\n"
-      << "      'Infinity': Infinity,\n"
-      << "      'Math': Math\n"
-      << "    }\n"
+  out << "return asmFunc({\n"
+      << "    'Int8Array': Int8Array,\n"
+      << "    'Int16Array': Int16Array,\n"
+      << "    'Int32Array': Int32Array,\n"
+      << "    'Uint8Array': Uint8Array,\n"
+      << "    'Uint16Array': Uint16Array,\n"
+      << "    'Uint32Array': Uint32Array,\n"
+      << "    'Float32Array': Float32Array,\n"
+      << "    'Float64Array': Float64Array,\n"
+      << "    'NaN': NaN,\n"
+      << "    'Infinity': Infinity,\n"
+      << "    'Math': Math\n"
       << "  },\n"
+      << "  asmLibraryArg,\n"
       << "  wasmMemory.buffer\n"
       << ")"
       << "\n"