Merge pull request #2016 from rfk/rfk/minify-names-separately

Split name-minification into a separate pass from registerization.

2 files changed, 103 insertions, 93 deletions

diff --git a/tools/js-optimizer.js b/tools/js-optimizer.js
index fc9e3002..4b1463eb 100644
--- a/tools/js-optimizer.js
+++ b/tools/js-optimizer.js
@@ -1773,9 +1773,7 @@ function ensureMinifiedNames(n) { // make sure the nth index in minifiedNames ex
   }
 }
 
-// Very simple 'registerization', coalescing of variables into a smaller number,
-// as part of minification. Globals-level minification began in a previous pass,
-// we receive extraInfo which tells us how to rename globals. (Only in asm.js.)
+// Very simple 'registerization', coalescing of variables into a smaller number.
 //
 // We do not optimize when there are switches, so this pass only makes sense with
 // relooping.
@@ -1811,6 +1809,7 @@ function registerize(ast) {
     // Replace all var definitions with assignments; we will add var definitions at the top after we registerize
     // We also mark local variables - i.e., having a var definition
     var localVars = {};
+    var allVars = {};
     var hasSwitch = false; // we cannot optimize variables if there is a switch, unless in asm mode
     traverse(fun, function(node, type) {
       if (type === 'var') {
@@ -1823,74 +1822,25 @@ function registerize(ast) {
         }
       } else if (type === 'switch') {
         hasSwitch = true;
+      } else if (type === 'name') {
+        allVars[node[1]] = 1;
       }
     });
     vacuum(fun);
-    if (extraInfo && extraInfo.globals) {
-      assert(asm);
-      var usedGlobals = {};
-      var nextLocal = 0;
-      // Minify globals using the mapping we were given
-      traverse(fun, function(node, type) {
-        if (type === 'name') {
-          var name = node[1];
-          var minified = extraInfo.globals[name];
-          if (minified) {
-            assert(!localVars[name], name); // locals must not shadow globals, or else we don't know which is which
-            if (localVars[minified]) {
-              // trying to minify a global into a name used locally. rename all the locals
-              var newName = '$_newLocal_' + (nextLocal++);
-              assert(!localVars[newName]);
-              if (params[minified]) {
-                params[newName] = 1;
-                delete params[minified];
-              }
-              localVars[newName] = 1;
-              delete localVars[minified];
-              asmData.vars[newName] = asmData.vars[minified];
-              delete asmData.vars[minified];
-              asmData.params[newName] = asmData.params[minified];
-              delete asmData.params[minified];
-              traverse(fun, function(node, type) {
-                if (type === 'name' && node[1] === minified) {
-                  node[1] = newName;
-                }
-              });
-              if (fun[2]) {
-                for (var i = 0; i < fun[2].length; i++) {
-                  if (fun[2][i] === minified) fun[2][i] = newName;
-                }
-              }
-            }
-            node[1] = minified;
-            usedGlobals[minified] = 1;
-          }
-        }
-      });
-      if (fun[1] in extraInfo.globals) { // if fun was created by a previous optimization pass, it will not be here
-        fun[1] = extraInfo.globals[fun[1]];
-        assert(fun[1]);
-      }
-      var nextRegName = 0;
-    }
     var regTypes = {};
     function getNewRegName(num, name) {
-      if (!asm) return 'r' + num;
-      var type = asmData.vars[name];
-      if (!extraInfo || !extraInfo.globals) {
-        var ret = (type ? 'd' : 'i') + num;
+      var ret;
+      if (!asm) {
+        ret = 'r' + num;
+      } else {
+        var type = asmData.vars[name];
+        ret = (type ? 'd' : 'i') + num;
         regTypes[ret] = type;
-        return ret;
       }
-      // find the next free minified name that is not used by a global that shows up in this function
-      while (1) {
-        ensureMinifiedNames(nextRegName);
-        var ret = minifiedNames[nextRegName++];
-        if (!usedGlobals[ret]) {
-          regTypes[ret] = type;
-          return ret;
-        }
+      if (ret in allVars) {
+        assert(ret in localVars, 'register shadows non-local name');
       }
+      return ret;
     }
     // Find the # of uses of each variable.
     // While doing so, check if all a variable's uses are dominated in a simple
@@ -2111,33 +2061,6 @@ function registerize(ast) {
         }
       }
       denormalizeAsm(fun, finalAsmData);
-      if (extraInfo && extraInfo.globals) {
-        // minify in asm var definitions, that denormalizeAsm just generated
-        function minify(value) {
-          if (value && value[0] === 'call' && value[1][0] === 'name') {
-            var name = value[1][1];
-            var minified = extraInfo.globals[name];
-            if (minified) {
-              value[1][1] = minified;
-            }
-          }
-        }
-        var stats = fun[3];
-        for (var i = 0; i < stats.length; i++) {
-          var line = stats[i];
-          if (i >= fun[2].length && line[0] !== 'var') break; // when we pass the arg and var coercions, break
-          if (line[0] === 'stat') {
-            assert(line[1][0] === 'assign');
-            minify(line[1][3]);
-          } else {
-            assert(line[0] === 'var');
-            var pairs = line[1];
-            for (var j = 0; j < pairs.length; j++) {
-              minify(pairs[j][1]);
-            }
-          }
-        }
-      }
     }
   });
 }
@@ -2913,6 +2836,92 @@ function minifyGlobals(ast) {
   suffix = '// EXTRA_INFO:' + JSON.stringify(minified);
 }
 
+
+function minifyLocals(ast) {
+  assert(asm)
+  assert(extraInfo && extraInfo.globals)
+
+  traverseGeneratedFunctions(ast, function(fun, type) {
+
+    // Analyse the asmjs to figure out local variable names,
+    // but operate on the original source tree so that we don't
+    // miss any global names in e.g. variable initializers.
+    var asmData = normalizeAsm(fun); denormalizeAsm(fun, asmData);
+    var newNames = {};
+    var usedNames = {};
+
+    // Find all the globals that we need to minify using
+    // pre-assigned names.  Don't actually minify them yet
+    // as that might interfere with local variable names.
+    function isLocalName(name) {
+      return name in asmData.vars || name in asmData.params;
+    }
+    traverse(fun, function(node, type) {
+      if (type === 'name') {
+        var name = node[1];
+        if (!isLocalName(name)) {
+          var minified = extraInfo.globals[name];
+          if (minified){
+            newNames[name] = minified;
+            usedNames[minified] = 1;
+          }
+        }
+      }
+    });
+
+    // Traverse and minify all names.
+    // The first time we encounter a local name, we assign it a
+    // minified name that's not currently in use.  Allocating on
+    // demand means they're processed in a predicatable order,
+    // which is very handy for testing/debugging purposes.
+    var nextMinifiedName = 0;
+    function getNextMinifiedName() {
+      var minified;
+      while (1) {
+        ensureMinifiedNames(nextMinifiedName);
+        minified = minifiedNames[nextMinifiedName++];
+        // TODO: we can probably remove !isLocalName here
+        if (!usedNames[minified] && !isLocalName(minified)) {
+          return minified;
+        }
+      }
+    }
+    if (fun[1] in extraInfo.globals) {
+      fun[1] = extraInfo.globals[fun[1]];
+      assert(fun[1]);
+    }
+    if (fun[2]) {
+      for (var i = 0; i < fun[2].length; i++) {
+        var minified = getNextMinifiedName();
+        newNames[fun[2][i]] = minified;
+        fun[2][i] = minified;
+      }
+    }
+    traverse(fun[3], function(node, type) {
+      if (type === 'name') {
+        var name = node[1];
+        var minified = newNames[name];
+        if (minified) {
+          node[1] = minified;
+        } else if (isLocalName(name)) {
+          minified = getNextMinifiedName();
+          newNames[name] = minified;
+          node[1] = minified;
+        }
+      } else if (type === 'var') {
+        node[1].forEach(function(defn) {
+          var name = defn[0];
+          if (!(name in newNames)) {
+            newNames[name] = getNextMinifiedName();
+          }
+          defn[0] = newNames[name];
+        });
+      }
+    });
+
+  });
+}
+
 // Relocation pass for a shared module (for the functions part of the module)
 //
 // 1. Replace function names with alternate names as defined (to avoid colliding with
@@ -3984,6 +3993,7 @@ var passes = {
   eliminateMemSafe: eliminateMemSafe,
   aggressiveVariableElimination: aggressiveVariableElimination,
   minifyGlobals: minifyGlobals,
+  minifyLocals: minifyLocals,
   relocate: relocate,
   outline: outline,
   minifyWhitespace: function() { minifyWhitespace = true },
diff --git a/tools/js_optimizer.py b/tools/js_optimizer.py
index 71b6f377..e030b707 100644
--- a/tools/js_optimizer.py
+++ b/tools/js_optimizer.py
@@ -26,7 +26,7 @@ class Minifier:
   '''
     asm.js minification support. We calculate minification of
     globals here, then pass that into the parallel js-optimizer.js runners which
-    during registerize perform minification of locals.
+    perform minification of locals.
   '''
 
   def __init__(self, js, js_engine):
@@ -117,9 +117,9 @@ def run_on_js(filename, passes, js_engine, jcache, source_map=False, extra_info=
 
   know_generated = suffix or start_funcs >= 0
 
-  minify_globals = 'registerizeAndMinify' in passes and 'asm' in passes
+  minify_globals = 'minifyNames' in passes and 'asm' in passes
   if minify_globals:
-    passes = map(lambda p: p if p != 'registerizeAndMinify' else 'registerize', passes)
+    passes = map(lambda p: p if p != 'minifyNames' else 'minifyLocals', passes)
     start_asm = js.find(start_asm_marker)
     end_asm = js.rfind(end_asm_marker)
     assert (start_asm >= 0) == (end_asm >= 0)