Use short-hand update syntax for simple updating definitions.

lib/compiler/implementation/ssa/codegen.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 class SsaCodeGeneratorTask extends CompilerTask {
   final JavaScriptBackend backend;
   SsaCodeGeneratorTask(JavaScriptBackend backend)
       : this.backend = backend,
         super(backend.compiler);
   String get name() => 'SSA code generator';
@@ skipping 468 matching lines @@
       declaredVariables.add(variableName);
       buffer.add("var ");
     }
     buffer.add(variableName);
   }
 
   void declareInstruction(HInstruction instruction) {
     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);
     }
   }
 
   void use(HInstruction argument, int expectedPrecedenceForArgument) {
     if (isGenerateAtUseSite(argument)) {
       visit(argument, expectedPrecedenceForArgument);
     } else if (argument is HCheck) {
       HCheck check = argument;
@@ skipping 383 matching lines @@
     }
     // Flow based traversal.
     if (node.isLoopHeader() &&
         node.loopInformation.loopBlockInformation !== currentBlockInformation) {
       beginLoop(node);
     }
     iterateBasicBlock(node);
   }
 
   void emitAssignment(String destination, String source) {
-    if (isGeneratingExpression()) { 
+    if (isGeneratingExpression()) {
       addExpressionSeparator();
     } else {
       addIndentation();
     }
     declareVariable(destination);
     buffer.add(' = $source');
     if (!isGeneratingExpression()) {
       buffer.add(';\n');
     }
   }
@@ skipping 1597 matching lines @@
       startBailoutSwitch();
     }
   }
 
   void endLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
     if (labeledBlockInfo.body.start.hasGuards()) {
       endBailoutSwitch();
     }
   }
 }