Added VariableUse expression to tree IR.

pkg/compiler/lib/src/tree_ir/optimization/copy_propagator.dart

Index: pkg/compiler/lib/src/tree_ir/optimization/copy_propagator.dart
diff --git a/pkg/compiler/lib/src/tree_ir/optimization/copy_propagator.dart b/pkg/compiler/lib/src/tree_ir/optimization/copy_propagator.dart
index a2828b697d175d31114ec71987939288743b0405..3cd05227e96e8b3d2e61978f8918ec63eef246ef 100644
--- a/pkg/compiler/lib/src/tree_ir/optimization/copy_propagator.dart
+++ b/pkg/compiler/lib/src/tree_ir/optimization/copy_propagator.dart
@@ -112,7 +112,8 @@ class CopyPropagator extends RecursiveVisitor with PassMixin {
     // Remove assignments of form w := v from the move maps.

[Kevin Millikin (Google), 2015/02/26 12:43:18]

It looks like we remove variable's that are read and variables that are written
for slightly different reasons.  We remove the assignment in '... w ... w := v'
because adding an assignment to w before that would add one that reaches the
read of w.  And we remove the second assignment in '... w := v0 ... w := v1'
because adding an assignment to w before that would add one that doesn't reach
the use (formerly v1).

I think that would be clearer if the was something like 'removeAssignment' or
'invalidateAssignment' and it was called directly with a comment explaining why
at the two call sites.  (I'm not a fan of visitVariable :) )

[asgerf, 2015/02/27 12:05:19]

Completely agree.

     Assign movingAssignment = inverseMove.remove(variable);
     if (movingAssignment != null) {
-      move.remove(movingAssignment.definition);
+      VariableUse def = movingAssignment.definition;
+      move.remove(def.variable);
     }
   }
 
@@ -139,16 +140,14 @@ class CopyPropagator extends RecursiveVisitor with PassMixin {
       // Make w := w.
       // We can't remove the statement from here because we don't have
       // parent pointers. So just make it a no-op so it can be removed later.
-      movingAssign.definition = w;
+      movingAssign.definition = new VariableUse(w);
 
       // The intermediate variable 'v' should now be orphaned, so don't bother
       // updating its read/write counters.
-      // Due to the nop trick, the variable 'w' now has one additional read
-      // and write.
-      ++w.writeCount;
-      ++w.readCount;
+      // Due to the nop trick, the variable 'w' now has one additional write.

[Kevin Millikin (Google), 2015/02/26 12:43:18]

'The nop trick' is a bit confusing.  Isn't the extra write because in the case
where we return w and not v, we will add an assignment to w (in the caller of
copyPropagateVariable)?

Is it possible to make a setter for Assign.variable that will update the write
count when the variable is changed?

[asgerf, 2015/02/27 12:05:19]

Yeah, that's why I moved it closer to the return. I forgot to remove the
comment.

Using getters/setters to update reference counts might give a false sense of
security since it still does not decrement counts when a node gets orphaned.
Then we would need a call a destroy() method to indicate when a node has been
removed. At the end of the day, I don't think it buys us a lot. Or maybe I just
have a preference for working with "raw data" rather than "smart objects".

[Kevin Millikin (Google), 2015/02/27 12:17:18]

I'm on the fence.  It's definitely a downside of trying to be too smart that you
end up with a bunch of similarly-named helpers that do things just slightly
differently (like if you ever need a different order, or to not do some of the
updates, or whatever).

I'm satisfied with leaving this explicit.

 
       // Make w := EXPR
+      ++w.writeCount;
       return w;
     }
     return v;
@@ -163,11 +162,11 @@ class CopyPropagator extends RecursiveVisitor with PassMixin {
     // If this is a moving assignment w := v, with this being the only use of v,
     // try to propagate it backwards.  Do not propagate assignments where w
     // is from an outer function scope.
-    if (node.definition is Variable) {
-      Variable def = node.definition;
-      if (def.readCount == 1 &&
+    if (node.definition is VariableUse) {
+      VariableUse definition = node.definition;
+      if (definition.variable.readCount == 1 &&
           node.variable.host == currentElement) {
-        move[node.definition] = node;
+        move[definition.variable] = node;
         inverseMove[node.variable] = node;
       }
     }