dart2dart: Preserve variable names throughout the IR.

sdk/lib/_internal/compiler/implementation/dart_backend/dart_tree.dart

Index: sdk/lib/_internal/compiler/implementation/dart_backend/dart_tree.dart
diff --git a/sdk/lib/_internal/compiler/implementation/dart_backend/dart_tree.dart b/sdk/lib/_internal/compiler/implementation/dart_backend/dart_tree.dart
index 6067fa89e52b5e0ed059ac8bbba13ed1d8a91686..9ba5f8fdb996d8ca3f71a449d82bc22ab1c0d407 100644
--- a/sdk/lib/_internal/compiler/implementation/dart_backend/dart_tree.dart
+++ b/sdk/lib/_internal/compiler/implementation/dart_backend/dart_tree.dart
@@ -83,18 +83,12 @@ class Label {
  * Variables are [Expression]s.
  */
 class Variable extends Expression {
-  // A counter used to generate names.  The counter is reset to 0 for each
-  // function emitted.
-  static int counter = 0;
-  static String _newName() => 'v${counter++}';
-
+  /// Element used for synthesizing a name for the variable.
+  /// Different variables may have the same element. May be null.
   Element element;
-  String cachedName;
 
-  String get name {
-    if (cachedName != null) return cachedName;
-    return cachedName = ((element == null) ? _newName() : element.name);
-  }
+  int readCount = 0;
+  int writeCount = 0;

[Kevin Millikin (Google), 2014/06/10 11:33:01]

Other than incrementing and decrementing, we do not [yet] use this, right?

[asgerf, 2014/06/10 12:22:23]

Right.

 
   Variable(this.element);
 
@@ -271,9 +265,12 @@ class Assign extends Statement {
   Statement next;
   final Variable variable;
   Expression definition;
-  final bool hasExactlyOneUse;
 
-  Assign(this.variable, this.definition, this.next, this.hasExactlyOneUse);
+  Assign(this.variable, this.definition, this.next) {
+    ++variable.writeCount;
+  }
+
+  bool get hasExactlyOneUse => variable.readCount == 1;
 
   accept(Visitor visitor) => visitor.visitAssign(this);
 }
@@ -301,19 +298,12 @@ class Return extends Statement {
  * labeled statement's successor statement.
  */
 class Break extends Statement {
-  Label _target;
-
-  Label get target => _target;
-  void set target(Label newTarget) {
-    ++newTarget.breakCount;
-    --_target.breakCount;
-    _target = newTarget;
-  }
+  Label target;
 
   Statement get next => null;
   void set next(Statement s) => throw 'UNREACHABLE';
 
-  Break(this._target) {
+  Break(this.target) {
     ++target.breakCount;
   }
 
@@ -444,9 +434,9 @@ abstract class Visitor<S, E> {
 class Builder extends ir.Visitor<Node> {
   final dart2js.Compiler compiler;
 
-  // Uses of IR primitives are replaced with Tree variables.  This is the
-  // mapping from primitives to variables.
-  final Map<ir.Primitive, Variable> variables = <ir.Primitive, Variable>{};
+  /// Maps variable/parameter elements to the Tree variables that represent it.
+  final Map<Element, List<Variable>> element2variables =
+      <Element,List<Variable>>{};
 
   // Continuations with more than one use are replaced with Tree labels.  This
   // is the mapping from continuations to labels.
@@ -457,14 +447,47 @@ class Builder extends ir.Visitor<Node> {
 
   Builder(this.compiler);
 
+  /// Obtains the variable representing the given primitive. Returns null for
+  /// primitives that have no reference and do not need a variable.
+  Variable getVariable(ir.Primitive primitive) {
+    if (primitive.registerIndex == null) {
+      return null; // variable is unused
+    }
+    List<Variable> variables = element2variables[primitive.element];
+    if (variables == null) {
+      variables = <Variable>[];
+      element2variables[primitive.element] = variables;
+    }
+    while (variables.length <= primitive.registerIndex) {
+      variables.add(new Variable(primitive.element));
+    }
+    return variables[primitive.registerIndex];
+  }
+
+  /// Obtains a reference to the tree Variable corresponding to the IR primitive
+  /// referred to by [reference].
+  /// This increments the reference count for the given variable, so the
+  /// returned expression must be used in the tree.
+  Expression getVariableReference(ir.Reference reference) {
+    Variable variable = getVariable(reference.definition);
+    if (variable == null) {
+      compiler.internalError(
+          compiler.currentElement,
+          "Reference to ${reference.definition} has no register");
+    }
+    ++variable.readCount;
+    return variable;
+  }
+
   FunctionDefinition build(ir.FunctionDefinition node) {
+    new ir.RegisterAllocator().visit(node);
     visit(node);
     return function;
   }
 
   List<Expression> translateArguments(List<ir.Reference> args) {
     return new List<Expression>.generate(args.length,
-         (int index) => variables[args[index].definition]);
+         (int index) => getVariableReference(args[index]));
   }
 
   Statement buildParameterAssignments(
@@ -474,19 +497,27 @@ class Builder extends ir.Visitor<Node> {
     assert(parameters.length == arguments.length);
     Statement first, current;
     for (int i = 0; i < parameters.length; ++i) {
+      Expression argument = arguments[i];
       ir.Parameter parameter = parameters[i];
       Statement assignment;
-      if (parameter.hasAtLeastOneUse) {
-        assignment = new Assign(variables[parameter], arguments[i], null,
-            parameter.hasExactlyOneUse);
+      Variable variable = getVariable(parameter);
+      if (variable == null || variable == arguments[i]) {
+        if (argument is Variable) {
+          assignment = null; // ignore pure expression

[Kevin Millikin (Google), 2014/06/10 11:33:01]

=> // Ignore pure expression.
  // Kill unused argument.

[asgerf, 2014/06/10 12:22:23]

Done.

+          --argument.readCount; // kill unused argument
+        } else {
+          assignment = new ExpressionStatement(argument, null);
+        }
       } else {
-        assignment = new ExpressionStatement(arguments[i], null);
+        assignment = new Assign(variable, argument, null);
       }
 
-      if (first == null) {
-        current = first = assignment;
-      } else {
-        current = current.next = assignment;
+      if (assignment != null) {
+        if (first == null) {
+          current = first = assignment;
+        } else {
+          current = current.next = assignment;
+        }
       }
     }
 
@@ -502,22 +533,20 @@ class Builder extends ir.Visitor<Node> {
     returnContinuation = node.returnContinuation;
     List<Variable> parameters = <Variable>[];
     for (ir.Parameter p in node.parameters) {
-      Variable parameter = new Variable(p.element);
+      Variable parameter = getVariable(p);
+      assert(parameter != null);
       parameters.add(parameter);
-      variables[p] = parameter;
     }
     function = new FunctionDefinition(parameters, visit(node.body));
     return null;
   }
 
   Statement visitLetPrim(ir.LetPrim node) {
-    // LetPrim is translated to LetVal.
+    // LetPrim is translated to Assign.
     Expression definition = visit(node.primitive);
-    if (node.primitive.hasAtLeastOneUse) {
-      Variable variable = new Variable(null);
-      variables[node.primitive] = variable;
-      return new Assign(variable, definition, visit(node.body),
-          node.primitive.hasExactlyOneUse);
+    Variable variable = getVariable(node.primitive);
+    if (variable != null) { // variable is null if primitive is unused

[Kevin Millikin (Google), 2014/06/10 11:33:01]

==> // Variable ... unused.

[asgerf, 2014/06/10 12:22:23]

Done.

+      return new Assign(variable, definition, node.body.accept(this));

[Kevin Millikin (Google), 2014/06/10 11:33:01]

visit(node.body)

[asgerf, 2014/06/10 12:22:23]

Done.

     } else if (node.primitive is ir.Constant) {
       // TODO(kmillikin): Implement more systematic treatment of pure CPS
       // values (e.g., as part of a shrinking reductions pass).
@@ -533,9 +562,6 @@ class Builder extends ir.Visitor<Node> {
       label = new Label();
       labels[node.continuation] = label;
     }
-    node.continuation.parameters.forEach((p) {
-        if (p.hasAtLeastOneUse) variables[p] = new Variable(null);
-    });
     Statement body = visit(node.body);
     // The continuation's body is not always translated directly here because
     // it may have been already translated:
@@ -565,7 +591,7 @@ class Builder extends ir.Visitor<Node> {
   }
 
   Statement visitInvokeMethod(ir.InvokeMethod node) {
-    Variable receiver = variables[node.receiver.definition];
+    Expression receiver = getVariableReference(node.receiver);
     List<Expression> arguments = translateArguments(node.arguments);
     Expression invoke = new InvokeMethod(receiver, node.selector, arguments);
     ir.Continuation cont = node.continuation.definition;
@@ -617,7 +643,7 @@ class Builder extends ir.Visitor<Node> {
     ir.Continuation cont = node.continuation.definition;
     if (cont == returnContinuation) {
       assert(node.arguments.length == 1);
-      return new Return(variables[node.arguments[0].definition]);
+      return new Return(getVariableReference(node.arguments[0]));
     } else {
       List<Expression> arguments = translateArguments(node.arguments);
       return buildParameterAssignments(cont.parameters, arguments,
@@ -694,7 +720,7 @@ class Builder extends ir.Visitor<Node> {
   }
 
   Expression visitIsTrue(ir.IsTrue node) {
-    return variables[node.value.definition];
+    return getVariableReference(node.value);
   }
 }
 
@@ -940,7 +966,7 @@ class StatementRewriter extends Visitor<Statement, Expression> {
       Break next = node.next;
       Label newTarget = redirect(next.target);
       labelRedirects[node.label] = newTarget;
-      newTarget.breakCount += node.label.breakCount;
+      newTarget.breakCount += node.label.breakCount - 1;
       node.label.breakCount = 0;
       Statement result = visitStatement(node.body);
       labelRedirects.remove(node.label); // Save some space.
@@ -1065,10 +1091,10 @@ class StatementRewriter extends Visitor<Statement, Expression> {
     if (s is Assign && t is Assign && s.variable == t.variable) {
       Statement next = combineStatements(s.next, t.next);
       if (next != null) {
+        --t.variable.writeCount; // Two assignments become one
         return new Assign(s.variable,
                           combine(s.definition, t.definition),
-                          next,
-                          s.hasExactlyOneUse);
+                          next);
       }
     }
     if (s is ExpressionStatement && t is ExpressionStatement) {
@@ -1091,24 +1117,27 @@ class StatementRewriter extends Visitor<Statement, Expression> {
       --t.target.breakCount; // Two breaks become one.
       return s;
     }
-    if (s is Return && t is Return && equivalentExpressions(s.value, t.value)) {
-      return s;
+    if (s is Return && t is Return) {
+      Expression e = combineExpressions(s.value, t.value);
+      if (e != null) {
+        return new Return(e);
+      }
     }
     return null;
   }
 
-  /// True if the two expressions both syntactically and semantically
-  /// equivalent.
-  static bool equivalentExpressions(Expression e1, Expression e2) {
-    if (e1 == e2) { // Detect same variable reference
-      // TODO(asgerf): This might turn the variable into a single-use,
-      // but we currently don't discover this.
-      return true;
+  /// Returns an expression equivalent to both [e1] and [e2].
+  /// If non-null is returned, the caller must discard [e1] and [e2] and use
+  /// the resulting expression in the tree.
+  static Expression combineExpressions(Expression e1, Expression e2) {
+    if (e1 is Variable && e1 == e2) {
+      --e1.readCount; // Two references become one.
+      return e1;
     }
-    if (e1 is Constant && e2 is Constant) {
-      return e1.value == e2.value;
+    if (e1 is Constant && e2 is Constant && e1.value == e2.value) {
+      return e1;
     }
-    return false;
+    return null;
   }
 
   /// Try to collapse nested ifs using && and || expressions.