merge loop and helper variables when their use ranges do not overlap

3 files changed, 99 insertions, 15 deletions

diff --git a/tools/eliminator/asm-eliminator-test-output.js b/tools/eliminator/asm-eliminator-test-output.js
index bf4b111f..50bd8f49 100644
--- a/tools/eliminator/asm-eliminator-test-output.js
+++ b/tools/eliminator/asm-eliminator-test-output.js
@@ -816,7 +816,7 @@ function selfAssign() {
 function elimOneLoopVar($argc, $argv) {
  $argc = $argc | 0;
  $argv = $argv | 0;
- var $arg$0 = 0, $call10 = Math_fround(0), $curri$012 = 0, $j$010 = 0, $ok$0 = 0, $primes$011 = 0, $retval$0 = 0, $vararg_buffer1 = 0, $j$010$looptemp = 0;
+ var $arg$0 = 0, $call10 = Math_fround(0), $curri$012 = 0, $j$010 = 0, $ok$0 = 0, $primes$011 = 0, $retval$0 = 0, $vararg_buffer1 = 0;
  $curri$012 = 2;
  $primes$011 = 0;
  while (1) {
@@ -825,12 +825,11 @@ function elimOneLoopVar($argc, $argv) {
    if ($call10 > Math_fround(+2)) {
     $j$010 = 2;
     while (1) {
-     $j$010$looptemp = $j$010;
-     $j$010 = $j$010 + 1 | 0;
-     if ((($curri$012 | 0) % ($j$010$looptemp | 0) & -1 | 0) == 0) {
+     if ((($curri$012 | 0) % ($j$010 | 0) & -1 | 0) == 0) {
       $ok$0 = 0;
       break L15;
      }
+     $j$010 = $j$010 + 1 | 0;
      if (!(Math_fround($j$010 | 0) < $call10)) {
       $ok$0 = 1;
       break;
@@ -895,10 +894,23 @@ function elimOneLoopVar4() {
  }
 }
 function elimOneLoopVarStillUsed() {
+ var $call10 = Math_fround(0), $curri$012 = 0, $j$010 = 0, $retval$0 = 0;
+ while (1) {
+  if ((($curri$012 | 0) % ($j$010 | 0) & -1 | 0) == 0) {
+   break;
+  }
+  $j$010 = $j$010 + 1 | 0;
+  if (!(Math_fround($j$010 | 0) < $call10)) {
+   break;
+  }
+ }
+ return $retval$0 | 0;
+}
+function elimOneLoopVarStillUsedSE() {
  var $call10 = Math_fround(0), $curri$012 = 0, $j$010 = 0, $retval$0 = 0, $j$010$looptemp = 0;
  while (1) {
   $j$010$looptemp = $j$010;
-  $j$010 = $j$010 + 1 | 0;
+  $j$010 = $j$010 + sideeffect() | 0;
   if ((($curri$012 | 0) % ($j$010$looptemp | 0) & -1 | 0) == 0) {
    break;
   }
diff --git a/tools/eliminator/asm-eliminator-test.js b/tools/eliminator/asm-eliminator-test.js
index 5bec0507..2613b356 100644
--- a/tools/eliminator/asm-eliminator-test.js
+++ b/tools/eliminator/asm-eliminator-test.js
@@ -1152,6 +1152,27 @@ function elimOneLoopVarStillUsed() {
  }
  return $retval$0 | 0;
 }
+function elimOneLoopVarStillUsedSE() {
+ var $0 = 0, $1 = 0, $arg$0 = 0, $arrayidx = 0, $call10 = Math_fround(0), $cmp = 0, $cmp11 = 0, $cmp119 = 0, $cmp12 = 0, $cmp7 = 0, $conv = 0, $conv8 = Math_fround(0), $conv9 = Math_fround(0), $curri$012 = 0, $inc = 0, $inc14$primes$0 = 0, $inc16 = 0, $j$010 = 0, $ok$0 = 0;
+ var $primes$011 = 0, $rem = 0, $retval$0 = 0, $sub = 0, $vararg_buffer1 = 0, label = 0, sp = 0;
+ while (1) {
+  $rem = ($curri$012 | 0) % ($j$010 | 0) & -1;
+  $cmp12 = ($rem | 0) == 0;
+  $inc = $j$010 + sideeffect() | 0; // side effect!
+  if ($cmp12) {
+   $ok$0 = 0;
+   break;
+  }
+  $conv8 = Math_fround($inc | 0);
+  $cmp11 = $conv8 < $call10;
+  if ($cmp11) {
+   $j$010 = $inc;
+  } else {
+   break;
+  }
+ }
+ return $retval$0 | 0;
+}
 function elimOneLoopVar5() {
  var $storemerge3$neg9 = 0, $18 = 0, $25 = 0, $26 = 0, $30 = 0, $jp = 0;
  $storemerge3$neg9 = -1;
@@ -1168,5 +1189,5 @@ function elimOneLoopVar5() {
   }
  }
 }
-// EMSCRIPTEN_GENERATED_FUNCTIONS: ["asm", "__Z11printResultPiS_j", "_segment_holding", "__ZN5identC2EiPKcPci", "_vec2Length", "exc", "label", "confuusion", "tempDouble", "_org_apache_harmony_luni_util_NumberConverter_freeFormat__", "__ZN23b2EdgeAndPolygonContact8EvaluateEP10b2ManifoldRK11b2TransformS4_", "_java_nio_charset_Charset_forNameInternal___java_lang_String", "looop2", "looop3", "looop4", "looop5", "looop6", "looop7", "looop8", "multiloop", "multiloop2", "tempDouble2", "watIf", "select2", "binary", "cute", "selfAssign", "elimOneLoopVar", "elimOneLoopVar2", "elimOneLoopVar3", "elimOneLoopVar4", "elimOneLoopVarStillUsed", "elimOneLoopVar5"]
+// EMSCRIPTEN_GENERATED_FUNCTIONS: ["asm", "__Z11printResultPiS_j", "_segment_holding", "__ZN5identC2EiPKcPci", "_vec2Length", "exc", "label", "confuusion", "tempDouble", "_org_apache_harmony_luni_util_NumberConverter_freeFormat__", "__ZN23b2EdgeAndPolygonContact8EvaluateEP10b2ManifoldRK11b2TransformS4_", "_java_nio_charset_Charset_forNameInternal___java_lang_String", "looop2", "looop3", "looop4", "looop5", "looop6", "looop7", "looop8", "multiloop", "multiloop2", "tempDouble2", "watIf", "select2", "binary", "cute", "selfAssign", "elimOneLoopVar", "elimOneLoopVar2", "elimOneLoopVar3", "elimOneLoopVar4", "elimOneLoopVarStillUsed", "elimOneLoopVarStillUsedSE", "elimOneLoopVar5"]
 
diff --git a/tools/js-optimizer.js b/tools/js-optimizer.js
index 699ea280..fe00254a 100644
--- a/tools/js-optimizer.js
+++ b/tools/js-optimizer.js
@@ -1034,6 +1034,27 @@ function hasSideEffects(node) { // this is 99% incomplete!
   }
 }
 
+// checks if a node has just basic operations, nothing with side effects nor that can notice side effects, which
+// implies we can move it around in the code
+function triviallySafeToMove(node, asmData) {
+  var ok = true;
+  traverse(node, function(node, type) {
+    switch (type) {
+      case 'stat': case 'binary': case 'unary-prefix': case 'assign': case 'num':
+        break;
+      case 'name':
+        if (!(node[1] in asmData.vars) && !(node[1] in asmData.params)) ok = false;
+        break;
+      case 'call':
+        if (callHasSideEffects(node)) ok = false;
+        break;
+      default:
+        ok = false;
+    }  
+  });
+  return ok;
+}
+
 // Clear out empty ifs and blocks, and redundant blocks/stats and so forth
 // Operates on generated functions only
 function vacuum(ast) {
@@ -3662,21 +3683,51 @@ function eliminate(ast, memSafe) {
               // if a loop variable is used after we assigned to the helper, we must save its value and use that.
               // (note that this can happen due to elimination, if we eliminate an expression containing the
               // loop var far down, past the assignment!)
-              var temp = looper + '$looptemp';
-              var looperUsed = false;
-              assert(!(temp in asmData.vars)); 
+              // first, see if the looper and helper overlap
+              var firstLooperUsage = -1;
+              var lastLooperUsage = -1;
+              var firstHelperUsage = -1;
+              var lastHelperUsage = -1;
               for (var i = found+1; i < stats.length; i++) {
                 var curr = i < stats.length-1 ? stats[i] : last[1]; // on the last line, just look in the condition
                 traverse(curr, function(node, type) {
-                  if (type === 'name' && node[1] === looper) {
-                    node[1] = temp;
-                    looperUsed = true;
+                  if (type === 'name') {
+                    if (node[1] === looper) {
+                      if (firstLooperUsage < 0) firstLooperUsage = i;
+                      lastLooperUsage = i;
+                    } else if (node[1] === helper) {
+                      if (firstHelperUsage < 0) firstHelperUsage = i;
+                      lastHelperUsage = i;
+                    }
                   }
                 });
               }
-              if (looperUsed) {
-                asmData.vars[temp] = asmData.vars[looper];
-                stats.splice(found, 0, ['stat', ['assign', true, ['name', temp], ['name', looper]]]);
+              if (firstLooperUsage >= 0) {
+                // the looper is used, we cannot simply merge the two variables
+                if ((firstHelperUsage < 0 || firstHelperUsage > lastLooperUsage) && lastLooperUsage+1 < stats.length && triviallySafeToMove(stats[found], asmData)) {
+                  // the helper is not used, or it is used after the last use of the looper, so they do not overlap,
+                  // and the last looper usage is not on the last line (where we could not append after it), and the
+                  // just move the looper definition to after the looper's last use
+                  stats.splice(lastLooperUsage+1, 0, stats[found]);
+                  stats.splice(found, 1);
+                } else {
+                  // they overlap, we can still proceed with the loop optimization, but we must introduce a
+                  // loop temp helper variable
+                  var temp = looper + '$looptemp';
+                  assert(!(temp in asmData.vars)); 
+                  for (var i = firstLooperUsage; i <= lastLooperUsage; i++) {
+                    var curr = i < stats.length-1 ? stats[i] : last[1]; // on the last line, just look in the condition
+                    traverse(curr, function(node, type) {
+                      if (type === 'name') {
+                        if (node[1] === looper) {
+                          node[1] = temp;
+                        }
+                      }
+                    });
+                  }
+                  asmData.vars[temp] = asmData.vars[looper];
+                  stats.splice(found, 0, ['stat', ['assign', true, ['name', temp], ['name', looper]]]);
+                }
               }
             }
             for (var l = 0; l < helpers.length; l++) {