optimize helper funcs (like i32-div) if created in asm2wasm, so they are consistently handled regardless of whether we optimize in parallel or not (#1208)

kripken · web-flow · commit fcbe7bff9e21 · 2017-10-04T16:45:40.000-07:00
diff --git a/src/asm2wasm.h b/src/asm2wasm.h
@@ -862,6 +862,7 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
       if (body[i][0] == DEFUN) numFunctions++;
     }
     optimizingBuilder = make_unique<OptimizingIncrementalModuleBuilder>(&wasm, numFunctions, passOptions, [&](PassRunner& passRunner) {
+      // addPrePasses
       if (debug) {
         passRunner.setDebug(true);
         passRunner.setValidateGlobally(false);
@@ -874,6 +875,18 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
       }
       // optimize relooper label variable usage at the wasm level, where it is easy
       passRunner.add("relooper-jump-threading");
+    }, [&]() {
+      // beforeGlobalOptimizations
+      // if we added any helper functions (like non-trapping i32-div, etc.), then those
+      // have not been optimized (the optimizing builder has just been fed the asm.js
+      // functions). Optimize those now. Typically there are very few, just do it
+      // sequentially.
+      PassRunner passRunner(&wasm, passOptions);
+      passRunner.addDefaultFunctionOptimizationPasses();
+      for (auto& pair : trappingFunctions.getFunctions()) {
+        auto* func = pair.second;
+        passRunner.runFunction(func);
+      }
     }, debug, false /* do not validate globally yet */);
   }
 
diff --git a/src/ast/trapping.h b/src/ast/trapping.h
@@ -85,6 +85,10 @@ class TrappingFunctionContainer {
     return wasm;
   }
 
+  std::map<Name, Function*>& getFunctions() {
+    return functions;
+  }
+
 private:
   std::map<Name, Function*> functions;
   std::map<Name, Import*> imports;
diff --git a/src/wasm-module-building.h b/src/wasm-module-building.h
@@ -75,6 +75,7 @@ class OptimizingIncrementalModuleBuilder {
   uint32_t numFunctions;
   PassOptions passOptions;
   std::function<void (PassRunner&)> addPrePasses;
+  std::function<void ()> beforeGlobalOptimizations;
   Function* endMarker;
   std::atomic<Function*>* list;
   uint32_t nextFunction; // only used on main thread
@@ -90,8 +91,13 @@ class OptimizingIncrementalModuleBuilder {
 public:
   // numFunctions must be equal to the number of functions allocated, or higher. Knowing
   // this bounds helps avoid locking.
-  OptimizingIncrementalModuleBuilder(Module* wasm, Index numFunctions, PassOptions passOptions, std::function<void (PassRunner&)> addPrePasses, bool debug, bool validateGlobally)
-      : wasm(wasm), numFunctions(numFunctions), passOptions(passOptions), addPrePasses(addPrePasses), endMarker(nullptr), list(nullptr), nextFunction(0),
+  OptimizingIncrementalModuleBuilder(Module* wasm, Index numFunctions, PassOptions passOptions,
+      std::function<void (PassRunner&)> addPrePasses,
+      std::function<void ()> beforeGlobalOptimizations,
+      bool debug, bool validateGlobally)
+      : wasm(wasm), numFunctions(numFunctions), passOptions(passOptions),
+        addPrePasses(addPrePasses), beforeGlobalOptimizations(beforeGlobalOptimizations),
+        endMarker(nullptr), list(nullptr), nextFunction(0),
         numWorkers(0), liveWorkers(0), activeWorkers(0), availableFuncs(0), finishedFuncs(0),
         finishing(false), debug(debug), validateGlobally(validateGlobally) {
 
@@ -165,14 +171,13 @@ class OptimizingIncrementalModuleBuilder {
       }
       addPrePasses(passRunner);
       passRunner.addDefaultFunctionOptimizationPasses();
-      passRunner.addDefaultGlobalOptimizationPostPasses();
       passRunner.run();
-      return;
+    } else {
+      DEBUG_THREAD("finish()ing");
+      assert(nextFunction == numFunctions);
+      wakeAllWorkers();
+      waitUntilAllFinished();
     }
-    DEBUG_THREAD("finish()ing");
-    assert(nextFunction == numFunctions);
-    wakeAllWorkers();
-    waitUntilAllFinished();
     optimizeGlobally();
     // TODO: clear side thread allocators from module allocator, as these threads were transient
   }
@@ -225,9 +230,11 @@ class OptimizingIncrementalModuleBuilder {
   }
 
   void optimizeGlobally() {
+    beforeGlobalOptimizations();
     PassRunner passRunner(wasm, passOptions);
     passRunner.addDefaultGlobalOptimizationPostPasses();
     passRunner.run();
+
   }
 
   // worker code
diff --git a/test/unit.asm.js b/test/unit.asm.js
@@ -150,6 +150,11 @@ function asm(global, env, buffer) {
     x = (4294967295 / 2)&-1;
     return x | 0;
   }
+  function trapping_sint_div_s() {
+    var x = 0;
+    x = (-2147483648 / -1);
+    return x | 0;
+  }
   function fr(x) {
     x = Math_fround(x);
     var y = Math_fround(0), z = 0.0;
@@ -790,6 +795,6 @@ function asm(global, env, buffer) {
   var FUNCTION_TABLE_vi = [ vi, vi, vi, vi, vi, vi, vi, vi ];
   var FUNCTION_TABLE_ii = [ ii ];
 
-  return { big_negative: big_negative, pick: forgetMe, pick: exportMe, doubleCompares: doubleCompares, intOps: intOps, conversions: conversions, switcher: switcher, frem: frem, frem_float: frem_float, big_uint_div_u: big_uint_div_u, fr: fr, negZero: negZero, neg: neg, smallCompare: smallCompare, cneg_nosemicolon: cneg_nosemicolon, forLoop: forLoop, ceiling_32_64: ceiling_32_64, aborts: aborts, continues: continues, bitcasts: bitcasts, recursiveBlockMerging: recursiveBlockMerging, lb: lb, zeroInit: zeroInit, phi: phi, smallIf: smallIf, dropCall: dropCall, useSetGlobal: useSetGlobal, usesSetGlobal2: usesSetGlobal2, breakThroughMany: breakThroughMany, ifChainEmpty: ifChainEmpty, heap8NoShift: heap8NoShift, conditionalTypeFun: conditionalTypeFun, loadSigned: loadSigned, globalOpts: globalOpts, dropCallImport: dropCallImport, loophi: loophi, loophi2: loophi2, loophi2b: loophi2b, relooperJumpThreading: relooperJumpThreading, relooperJumpThreading__ZN4game14preloadweaponsEv: relooperJumpThreading__ZN4game14preloadweaponsEv, __Z12multi_varargiz: __Z12multi_varargiz, jumpThreadDrop: jumpThreadDrop, dropIgnoredImportInIf: dropIgnoredImportInIf, dropIgnoredImportsInIf: dropIgnoredImportsInIf, relooperJumpThreading_irreducible: relooperJumpThreading_irreducible, store_fround: store_fround, exportedNumber: 42, relocatableAndModules: relocatableAndModules, exported_f32_user: exported_f32_user, keepAlive: keepAlive };
+  return { big_negative: big_negative, pick: forgetMe, pick: exportMe, doubleCompares: doubleCompares, intOps: intOps, conversions: conversions, switcher: switcher, frem: frem, frem_float: frem_float, big_uint_div_u: big_uint_div_u, trapping_sint_div_s: trapping_sint_div_s, fr: fr, negZero: negZero, neg: neg, smallCompare: smallCompare, cneg_nosemicolon: cneg_nosemicolon, forLoop: forLoop, ceiling_32_64: ceiling_32_64, aborts: aborts, continues: continues, bitcasts: bitcasts, recursiveBlockMerging: recursiveBlockMerging, lb: lb, zeroInit: zeroInit, phi: phi, smallIf: smallIf, dropCall: dropCall, useSetGlobal: useSetGlobal, usesSetGlobal2: usesSetGlobal2, breakThroughMany: breakThroughMany, ifChainEmpty: ifChainEmpty, heap8NoShift: heap8NoShift, conditionalTypeFun: conditionalTypeFun, loadSigned: loadSigned, globalOpts: globalOpts, dropCallImport: dropCallImport, loophi: loophi, loophi2: loophi2, loophi2b: loophi2b, relooperJumpThreading: relooperJumpThreading, relooperJumpThreading__ZN4game14preloadweaponsEv: relooperJumpThreading__ZN4game14preloadweaponsEv, __Z12multi_varargiz: __Z12multi_varargiz, jumpThreadDrop: jumpThreadDrop, dropIgnoredImportInIf: dropIgnoredImportInIf, dropIgnoredImportsInIf: dropIgnoredImportsInIf, relooperJumpThreading_irreducible: relooperJumpThreading_irreducible, store_fround: store_fround, exportedNumber: 42, relocatableAndModules: relocatableAndModules, exported_f32_user: exported_f32_user, keepAlive: keepAlive };
 }
 
diff --git a/test/unit.fromasm b/test/unit.fromasm
@@ -36,6 +36,7 @@
  (export "frem" (func $frem))
  (export "frem_float" (func $legalstub$frem_float))
  (export "big_uint_div_u" (func $big_uint_div_u))
+ (export "trapping_sint_div_s" (func $trapping_sint_div_s))
  (export "fr" (func $legalstub$fr))
  (export "negZero" (func $negZero))
  (export "neg" (func $neg))
@@ -254,6 +255,9 @@
  (func $big_uint_div_u (result i32)
   (i32.const 2147483647)
  )
+ (func $trapping_sint_div_s (result i32)
+  (i32.const 0)
+ )
  (func $fr (param $0 f32)
   (nop)
  )
diff --git a/test/unit.fromasm.clamp b/test/unit.fromasm.clamp
@@ -34,6 +34,7 @@
  (export "frem" (func $frem))
  (export "frem_float" (func $legalstub$frem_float))
  (export "big_uint_div_u" (func $big_uint_div_u))
+ (export "trapping_sint_div_s" (func $trapping_sint_div_s))
  (export "fr" (func $legalstub$fr))
  (export "negZero" (func $negZero))
  (export "neg" (func $neg))
@@ -302,6 +303,9 @@
  (func $big_uint_div_u (result i32)
   (i32.const 2147483647)
  )
+ (func $trapping_sint_div_s (result i32)
+  (i32.const 0)
+ )
  (func $fr (param $0 f32)
   (nop)
  )
diff --git a/test/unit.fromasm.clamp.no-opts b/test/unit.fromasm.clamp.no-opts
@@ -41,6 +41,7 @@
  (export "frem" (func $frem))
  (export "frem_float" (func $legalstub$frem_float))
  (export "big_uint_div_u" (func $big_uint_div_u))
+ (export "trapping_sint_div_s" (func $trapping_sint_div_s))
  (export "fr" (func $legalstub$fr))
  (export "negZero" (func $negZero))
  (export "neg" (func $neg))
@@ -477,6 +478,43 @@
    (get_local $x)
   )
  )
+ (func $i32s-div (param $0 i32) (param $1 i32) (result i32)
+  (if (result i32)
+   (i32.eqz
+    (get_local $1)
+   )
+   (i32.const 0)
+   (if (result i32)
+    (i32.and
+     (i32.eq
+      (get_local $0)
+      (i32.const -2147483648)
+     )
+     (i32.eq
+      (get_local $1)
+      (i32.const -1)
+     )
+    )
+    (i32.const 0)
+    (i32.div_s
+     (get_local $0)
+     (get_local $1)
+    )
+   )
+  )
+ )
+ (func $trapping_sint_div_s (result i32)
+  (local $x i32)
+  (set_local $x
+   (call $i32s-div
+    (i32.const -2147483648)
+    (i32.const -1)
+   )
+  )
+  (return
+   (get_local $x)
+  )
+ )
  (func $fr (param $x f32)
   (local $y f32)
   (local $z f64)
diff --git a/test/unit.fromasm.imprecise b/test/unit.fromasm.imprecise
@@ -33,6 +33,7 @@
  (export "frem" (func $frem))
  (export "frem_float" (func $legalstub$frem_float))
  (export "big_uint_div_u" (func $big_uint_div_u))
+ (export "trapping_sint_div_s" (func $trapping_sint_div_s))
  (export "fr" (func $legalstub$fr))
  (export "negZero" (func $negZero))
  (export "neg" (func $neg))
@@ -240,6 +241,12 @@
  (func $big_uint_div_u (result i32)
   (i32.const 2147483647)
  )
+ (func $trapping_sint_div_s (result i32)
+  (i32.div_s
+   (i32.const -2147483648)
+   (i32.const -1)
+  )
+ )
  (func $fr (param $0 f32)
   (nop)
  )
diff --git a/test/unit.fromasm.imprecise.no-opts b/test/unit.fromasm.imprecise.no-opts
@@ -41,6 +41,7 @@
  (export "frem" (func $frem))
  (export "frem_float" (func $legalstub$frem_float))
  (export "big_uint_div_u" (func $big_uint_div_u))
+ (export "trapping_sint_div_s" (func $trapping_sint_div_s))
  (export "fr" (func $legalstub$fr))
  (export "negZero" (func $negZero))
  (export "neg" (func $neg))
@@ -413,6 +414,18 @@
    (get_local $x)
   )
  )
+ (func $trapping_sint_div_s (result i32)
+  (local $x i32)
+  (set_local $x
+   (i32.div_s
+    (i32.const -2147483648)
+    (i32.const -1)
+   )
+  )
+  (return
+   (get_local $x)
+  )
+ )
  (func $fr (param $x f32)
   (local $y f32)
   (local $z f64)
diff --git a/test/unit.fromasm.no-opts b/test/unit.fromasm.no-opts
@@ -43,6 +43,7 @@
  (export "frem" (func $frem))
  (export "frem_float" (func $legalstub$frem_float))
  (export "big_uint_div_u" (func $big_uint_div_u))
+ (export "trapping_sint_div_s" (func $trapping_sint_div_s))
  (export "fr" (func $legalstub$fr))
  (export "negZero" (func $negZero))
  (export "neg" (func $neg))
@@ -429,6 +430,43 @@
    (get_local $x)
   )
  )
+ (func $i32s-div (param $0 i32) (param $1 i32) (result i32)
+  (if (result i32)
+   (i32.eqz
+    (get_local $1)
+   )
+   (i32.const 0)
+   (if (result i32)
+    (i32.and
+     (i32.eq
+      (get_local $0)
+      (i32.const -2147483648)
+     )
+     (i32.eq
+      (get_local $1)
+      (i32.const -1)
+     )
+    )
+    (i32.const 0)
+    (i32.div_s
+     (get_local $0)
+     (get_local $1)
+    )
+   )
+  )
+ )
+ (func $trapping_sint_div_s (result i32)
+  (local $x i32)
+  (set_local $x
+   (call $i32s-div
+    (i32.const -2147483648)
+    (i32.const -1)
+   )
+  )
+  (return
+   (get_local $x)
+  )
+ )
  (func $fr (param $x f32)
   (local $y f32)
   (local $z f64)
