Avoid inserting new temporaries because of HTypeConversion nodes.

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 {
   SsaCodeGeneratorTask(Compiler compiler) : super(compiler);
   String get name() => 'SSA code generator';
 
 
   String buildJavaScriptFunction(FunctionElement element,
@@ skipping 499 matching lines @@
       if (!isGeneratingDeclaration()) {
         delayedVarDecl = delayedVarDecl.prepend(variableName);
       }
     } else {
       buffer.add("var ");
       buffer.add(variableName);
     }
   }
 
   void define(HInstruction instruction) {
-    String name = temporary(instruction);
-    declareVariable(name);
-    buffer.add(" = ");
-    visit(instruction, JSPrecedence.ASSIGNMENT_PRECEDENCE);
+    bool needsVar = !instruction.usedBy.isEmpty();
+    if (needsVar) {
+      if (instruction.returnsInput()) {

[Lasse Reichstein Nielsen, 2012/05/08 12:39:08]

Use single if with '&&'.

[ngeoffray, 2012/05/08 16:09:12]

Done.

+        HInstruction input = instruction.input;
+        // We only need a new var if [input] is generated at use site
+        // but is not a trivial code motion invariant instruction (eg

[floitsch, 2012/05/08 13:21:42]

"like for". There is enough space.

[ngeoffray, 2012/05/08 16:09:12]

Done.

+        // parameters or this).
+        //
+        // For example:
+        // Foo a = this;
+        // print(a);
+        // print(a);
+        //
+        // In checked mode does not need a new variable:

[floitsch, 2012/05/08 13:21:42]

In checked mode no new variable is needed:

[ngeoffray, 2012/05/08 16:09:12]

Done.

+        // FooTypeCheck(this);
+        // print(this);
+        // print(this);
+        //
+        // But for this example:
+        // Foo a = foo();
+        // print(a);
+        // print(a);
+        //
+        // We need a new variable:
+        // var a = FooTypeCheck(foo());
+        // print(a);
+        // print(a);
+        needsVar = isGenerateAtUseSite(input) && !input.isCodeMotionInvariant();

[floitsch, 2012/05/08 13:21:42]

If I understand correctly it is crucial that 'define' and 'use' agree on when to
use a var, and when not.
Therefore move the check into a separate function.

[ngeoffray, 2012/05/08 16:09:12]

Done.

+      }
+    }
+    if (needsVar) {
+      String name = temporary(instruction);
+      declareVariable(name);
+      buffer.add(" = ");
+      visit(instruction, JSPrecedence.ASSIGNMENT_PRECEDENCE);
+    } else {
+      visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
+    }
   }
 
   void use(HInstruction argument, int expectedPrecedenceForArgument) {
-    if (isGenerateAtUseSite(argument)) {
+    if (argument.returnsInput()) {
+      HInstruction input = argument.input;
+      if (isGenerateAtUseSite(argument) && isGenerateAtUseSite(input)) {
+        // If both instructions can be generated at use site, we can
+        // just visit [argument].
+        //
+        // For example:
+        // Foo a = foo();
+        // print(a);
+        //
+        // In checked mode will turn into:
+        // print(FooTypeCheck(foo()));
+        visit(argument, expectedPrecedenceForArgument);
+      } else if (isGenerateAtUseSite(input) && !input.isCodeMotionInvariant()) {

[Lasse Reichstein Nielsen, 2012/05/08 12:39:08]

Please explain the use of isCodeMotionInvariant.

[ngeoffray, 2012/05/08 16:09:12]

Done.

+        // If [argument] cannot be generated at use site, but [input]
+        // can, use the temporary of [argument].
+        //
+        // For example:
+        // Foo a = foo();
+        // print(a);
+        // print(a);
+        //
+        // In checked mode will turn into:
+        // var a = FooTypeCheck(foo());
+        // print(a);
+        // print(a);
+        buffer.add(temporary(argument));
+      } else {
+        // Otherwise we just use [input].

[Lasse Reichstein Nielsen, 2012/05/08 12:39:08]

Please give example of output here too.
What happens to argument? Is it not generated at all?

[ngeoffray, 2012/05/08 16:09:12]

Done.

+        use(input, expectedPrecedenceForArgument);
+      }
+    } else if (isGenerateAtUseSite(argument)) {
       visit(argument, expectedPrecedenceForArgument);
-    } else if (argument is HIntegerCheck) {
-      HIntegerCheck instruction = argument;
-      use(instruction.value, expectedPrecedenceForArgument);
-    } else if (argument is HBoundsCheck) {
-      HBoundsCheck instruction = argument;
-      use(instruction.index, expectedPrecedenceForArgument);
-    } else if (argument is HTypeGuard) {
-      HTypeGuard instruction = argument;
-      use(instruction.guarded, expectedPrecedenceForArgument);
     } else {
       buffer.add(temporary(argument));
     }
   }
 
   visit(HInstruction node, int expectedPrecedenceForNode) {
     int oldPrecedence = this.expectedPrecedence;
     this.expectedPrecedence = expectedPrecedenceForNode;
     node.accept(this);
     this.expectedPrecedence = oldPrecedence;
@@ skipping 508 matching lines @@
     }
   }
 
   void iterateBasicBlock(HBasicBlock node) {
     HInstruction instruction = node.first;
     while (instruction != null) {
       if (instruction === node.last) {
         assignPhisOfAllSuccessors(node);
       }
 
-      if (instruction is HGoto || instruction is HExit || instruction is HTry) {
-        visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
-        return;
-      } else if (!isGenerateAtUseSite(instruction)) {
-        if (instruction is !HIf
-            && instruction is !HTypeGuard
-            && instruction is !HLoopBranch
-            && !isGeneratingExpression()) {
-          addIndentation();
+      if (isGenerateAtUseSite(instruction)) {
+        if (instruction is HIf) {
+          HIf hif = instruction;
+          // The "if" is implementing part of a logical expression.
+          // Skip directly forward to to its latest successor, since everything
+          // in-between must also be generateAtUseSite.
+          assert(hif.trueBranch.id < hif.falseBranch.id);
+          visitBasicBlock(hif.falseBranch);
         }
-        if (isGeneratingExpression()) {
+      } else if (instruction is HControlFlow) {
+        if (instruction is HLoopBranch && isGeneratingExpression()) {
           addExpressionSeparator();
         }
-        if (instruction.usedBy.isEmpty()
-            || instruction is HTypeGuard
-            || instruction is HCheck) {
-          visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
-        } else {
-          define(instruction);
-        }
-        // Control flow instructions, and some other instructions,
-        // know how to handle ';'.
-        if (instruction is !HControlFlow
-            && instruction is !HTypeGuard
-            && !isGeneratingExpression()) {
-          buffer.add(';\n');
-        }
-      } else if (instruction is HIf) {
-        HIf hif = instruction;
-        // The "if" is implementing part of a logical expression.
-        // Skip directly forward to to its latest successor, since everything
-        // in-between must also be generateAtUseSite.
-        assert(hif.trueBranch.id < hif.falseBranch.id);
-        visitBasicBlock(hif.falseBranch);
-        return;
+        visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
+      } else if (instruction is HTypeGuard) {
+        visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
+      } else {
+        isGeneratingExpression() ? addExpressionSeparator() : addIndentation();

[Lasse Reichstein Nielsen, 2012/05/08 12:39:08]

Use if. Please don't (try) to be clever with the conditional operator. It hurts.
If it's too long, it seems like something that can be extracted into a reusable
method.

[ngeoffray, 2012/05/08 16:09:12]

Done.

+        define(instruction);
+        if (!isGeneratingExpression()) buffer.add(';\n');
       }
       instruction = instruction.next;
     }
   }
 
   visitInvokeBinary(HInvokeBinary node, String op) {
     if (node.builtin) {
       JSBinaryOperatorPrecedence operatorPrecedences = JSPrecedence.binary[op];
       beginExpression(operatorPrecedences.precedence);
       use(node.left, operatorPrecedences.left);
@@ skipping 500 matching lines @@
     String operation = logicalOperations[node];
     if (operation !== null) {
       emitLogicalOperation(node, operation);
     } else {
       HPhi canonicalPhi = phiEquivalence.getRepresentative(node);
       buffer.add('${temporary(canonicalPhi)}');
     }
   }
 
   visitReturn(HReturn node) {
+    addIndentation();
     assert(node.inputs.length == 1);
     HInstruction input = node.inputs[0];
     if (input.isConstantNull()) {
       buffer.add('return;\n');
     } else {
       buffer.add('return ');
       use(node.inputs[0], JSPrecedence.EXPRESSION_PRECEDENCE);
       buffer.add(';\n');
     }
   }
 
   visitThis(HThis node) {
     buffer.add('this');
   }
 
   visitThrow(HThrow node) {
+    addIndentation();
     if (node.isRethrow) {
       buffer.add('throw ');
       use(node.inputs[0], JSPrecedence.EXPRESSION_PRECEDENCE);
     } else {
       generateThrowWithHelper('captureStackTrace', node.inputs[0]);
     }
     buffer.add(';\n');
   }
 
   visitBoundsCheck(HBoundsCheck node) {
@@ skipping 379 matching lines @@
         buffer.add(".${name.slowToString()} === '${arguments.head}'");
         endExpression(JSPrecedence.LOGICAL_AND_PRECEDENCE);
       });
     }
     if (node.nullOk) {
       endExpression(JSPrecedence.LOGICAL_OR_PRECEDENCE);
     }
   }
 
   void visitTypeConversion(HTypeConversion node) {
+    HInstruction input = node.inputs[0];

[Lasse Reichstein Nielsen, 2012/05/08 12:39:08]

node.input?
Why extract input here, when it's only used once on two separate branches later?

[ngeoffray, 2012/05/08 16:09:12]

Done.

     if (node.checked) {
       Element element = node.type.computeType(compiler).element;
       compiler.registerIsCheck(element);
       SourceString helper;
       String additionalArgument;
       bool nativeCheck =
           compiler.emitter.nativeEmitter.requiresNativeIsCheck(element);
       beginExpression(JSPrecedence.CALL_PRECEDENCE);
 
       if (element == compiler.stringClass) {
@@ skipping 27 matching lines @@
         } else if (nativeCheck) {
           helper = const SourceString('callTypeCheck');
         } else {
           helper = const SourceString('propertyTypeCheck');
         }
       }
       Element helperElement = compiler.findHelper(helper);
       compiler.registerStaticUse(helperElement);
       buffer.add(compiler.namer.isolateAccess(helperElement));
       buffer.add('(');
-      use(node.inputs[0], JSPrecedence.EXPRESSION_PRECEDENCE);
+      use(input, JSPrecedence.EXPRESSION_PRECEDENCE);
       if (additionalArgument !== null) buffer.add(", '$additionalArgument'");
       buffer.add(')');
       endExpression(JSPrecedence.CALL_PRECEDENCE);
     } else {
-      use(node.inputs[0], expectedPrecedence);
+      visit(input, expectedPrecedence);
     }
   }
 }
 
 class SsaOptimizedCodeGenerator extends SsaCodeGenerator {
   SsaOptimizedCodeGenerator(compiler, work, parameters, parameterNames)
     : super(compiler, work, parameters, parameterNames);
 
   void beginGraph(HGraph graph) {}
   void endGraph(HGraph graph) {}
@@ skipping 388 matching lines @@
       startBailoutSwitch();
     }
   }
 
   void endLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
     if (labeledBlockInfo.body.start.hasGuards()) {
       endBailoutSwitch();
     }
   }
 }