Use short-hand update syntax for simple updating definitions.

lib/compiler/implementation/ssa/codegen.dart

Index: lib/compiler/implementation/ssa/codegen.dart
diff --git a/lib/compiler/implementation/ssa/codegen.dart b/lib/compiler/implementation/ssa/codegen.dart
index a7ff2777c7b95b47f546c0068e4b92f052e21dcb..2a5740e0f5c53b5ceb48e51e09232a22f1ea39b3 100644
--- a/lib/compiler/implementation/ssa/codegen.dart
+++ b/lib/compiler/implementation/ssa/codegen.dart
@@ -486,11 +486,70 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
     declareVariable(variableNames.getName(instruction));
   }
 
+  // For simple updates of the form 'i = i op constant' generate
+  // 'i op= constant' instead.
+  bool handleSimpleUpdateDefinition(HInstruction instruction, String name) {
+    // If the variable is not declared the short update syntax cannot
+    // be used since it is a declaration and not an update.
+    if (!isVariableDeclared(name)) return false;

[ngeoffray, 2012/06/04 13:24:00]

I think you should check that it's not delayed either. It should be fine to use
i++ if it's delayed. Or maybe should we declare a variable as declared if it's
delayed? I think we're duplicating 'var' if we don't.

[Mads Ager (google), 2012/06/04 15:27:18]

Yes, went for marking a variable declared when we add it to the
delayedVariables.

+
+    // Extract the operation and whether or not it is commutative.

[kasperl, 2012/06/04 13:12:56]

I guess you considered moving some of this code to the nodes, but that you
prefer to keep it local to the codegen.

[Mads Ager (google), 2012/06/04 13:21:45]

Yes, I decided to keep it local since I only consider a few of the operations
here.

+    var operation;

[ngeoffray, 2012/06/04 13:24:00]

Please use the Operation class in lib/compiler/implementation/operations.dart to
check for commutativity and the operation's name.

[Mads Ager (google), 2012/06/04 15:27:18]

In principle that would be very nice. In practice the Operation class is not
very well suited for this. Add operations are not commutative when dealing with
strings so we need a special case for that. Not all operations that are
BinaryArithmetic operations and are builtin will work with this (such as
truncating divide). So, in the end we will end up with the same checks. I have
used the Operation class to get the name of the operation which makes the code
shorter, but I still need the other checks.

+    var isCommutative = instruction is HAdd || instruction is HMultiply;

[kasperl, 2012/06/04 13:12:56]

Make this start out by being false and push the update to true inside the
branches? Your call.

[Mads Ager (google), 2012/06/04 13:21:45]

Yes! Done.

+    if (instruction is HAdd) {
+      operation = '+';
+    } else if (instruction is HMultiply) {
+      operation = '*';
+    } else if (instruction is HSubtract) {
+      operation = '-';
+    } else if (instruction is HDivide) {
+      operation = '/';
+    } else {
+      return false;
+    }
+
+    // Is it a simple builtin operation involving constant numbers?
+    if (instruction.builtin && instruction.inputs.length == 3) {
+      var left = instruction.inputs[1];
+      var right = instruction.inputs[2];
+      if (left.isConstantNumber() && isCommutative) {
+        var tmp = right;
+        right = left;
+        left = tmp;
+      } else if (!right.isConstantNumber()) {
+        return false;
+      }
+      // Right is constant number.
+      var value = right.constant.value;
+      // Check that left is a phi with the same name as the definition
+      // and emit the short update definition if it is.
+      if (left is HPhi &&

[ngeoffray, 2012/06/04 13:24:00]

I think you can remove the check. You only need to check if
variableNames.getName(left) == name

[Mads Ager (google), 2012/06/04 15:27:18]

Great, thanks! 

+          variableNames.hasName(left) &&
+          variableNames.getName(left) == name) {
+        if ((operation == '+') && right.constant.value == 1) {

[kasperl, 2012/06/04 13:12:56]

Looks a bit fishy that you have () around one == (left) but not the other.

[Mads Ager (google), 2012/06/04 13:21:45]

Yes, that looks weird. Updated to consistently use parens.

+          buffer.add('++');
+          declareInstruction(instruction);
+        } else if ((operation == '-') && right.constant.value == 1) {

[kasperl, 2012/06/04 13:12:56]

Ditto.

[Mads Ager (google), 2012/06/04 13:21:45]

Thanks. Done.

+          buffer.add('--');
+          declareInstruction(instruction);
+        } else {
+          declareInstruction(instruction);
+          buffer.add(' ${operation}= ${value}');
+        }
+        return true;
+      }
+    }
+    return false;
+  }
+
   void define(HInstruction instruction) {
     if (instruction is !HCheck && variableNames.hasName(instruction)) {
-      declareInstruction(instruction);
-      buffer.add(" = ");
-      visit(instruction, JSPrecedence.ASSIGNMENT_PRECEDENCE);
+      if (!handleSimpleUpdateDefinition(instruction,
+                                        variableNames.getName(instruction))) {

[kasperl, 2012/06/04 13:12:56]

I would throw variableNames.getName(instruction) into a local to make it easier
to format.

[Mads Ager (google), 2012/06/04 13:21:45]

Done.

+        declareInstruction(instruction);
+        buffer.add(" = ");
+        visit(instruction, JSPrecedence.ASSIGNMENT_PRECEDENCE);
+      }
     } else {
       visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
     }
@@ -894,7 +953,7 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
   }
 
   void emitAssignment(String destination, String source) {
-    if (isGeneratingExpression()) { 
+    if (isGeneratingExpression()) {
       addExpressionSeparator();
     } else {
       addIndentation();