Compute liveness of instructions to get better and fewer variable names.

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 123 matching lines @@
    * or possibly several comma-separated expressions.
    * - [TYPE_DECLARATION] means that the graph can be generated as an
    * expression, and that it only generates expressions of the form
    *   variable = expression
    * which are also valid as parts of a "var" declaration.
    */
   static final int TYPE_STATEMENT = 0;
   static final int TYPE_EXPRESSION = 1;
   static final int TYPE_DECLARATION = 2;
 
-  static final String TEMPORARY_PREFIX = 't';
-
   final JavaScriptBackend backend;
   final WorkItem work;
   final StringBuffer buffer;
   final String parameters;
 
-  final Map<Element, String> parameterNames;
-  final Map<int, String> names;
-  final Set<String> usedNames;
-  final Set<HInstruction> declaredInstructions;
-  final Map<String, int> prefixes;
+  final Set<String> declaredVariables;

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

Can you add a doc-comment describing what this means (and ditto for
delayedVariableDeclarations, so the distinction is clear)

[ngeoffray, 2012/05/31 08:48:22]

Done.

   final Set<HInstruction> generateAtUseSite;
   final Map<HPhi, String> logicalOperations;
   final Map<Element, ElementAction> breakAction;
   final Map<Element, ElementAction> continueAction;
-  final Equivalence<HPhi> phiEquivalence;
+  final List<String> delayedVariablesDeclaration;
+  final Map<Element, String> parameterNames;
+  VariableNames variableNames;

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

And this one too, to explain the difference from the other ones.

[ngeoffray, 2012/05/31 08:48:22]

Done.

 
   Element equalsNullElement;
   Element boolifiedEqualsNullElement;
   int indent = 0;
   int expectedPrecedence = JSPrecedence.STATEMENT_PRECEDENCE;
   JSBinaryOperatorPrecedence unsignedShiftPrecedences;
   HGraph currentGraph;
   /**
    * Whether the code-generation should try to generate an expression
    * instead of a sequence of statements.
    */
   int generationState = STATE_STATEMENT;
   /**
    * While generating expressions, we can't insert variable declarations.
    * Instead we declare them at the end of the function

[floitsch, 2012/05/30 18:45:03]

Move this comment too.

[ngeoffray, 2012/05/31 08:12:46]

Done.

    */
-  Link<String> delayedVarDecl = const EmptyLink<String>();
   HBasicBlock currentBlock;
 
   // Records a block-information that is being handled specially.
   // Used to break bad recursion.
   HBlockInformation currentBlockInformation;
   // The subgraph is used to delimit traversal for some constructions, e.g.,
   // if branches.
   SubGraph subGraph;
 
   LibraryElement get currentLibrary() => work.element.getLibrary();
   Compiler get compiler() => backend.compiler;
   NativeEmitter get nativeEmitter() => backend.emitter.nativeEmitter;
   Enqueuer get world() => backend.compiler.enqueuer.codegen;
 
   bool isGenerateAtUseSite(HInstruction instruction) {
     return generateAtUseSite.contains(instruction);
   }
 
   SsaCodeGenerator(this.backend,
                    this.work,
                    this.parameters,
                    this.parameterNames)
-    : names = new Map<int, String>(),
-      prefixes = new Map<String, int>(),
-      usedNames = new Set<String>(),
-      declaredInstructions = new Set<HInstruction>(),
+    : declaredVariables = new Set<String>(),
+      delayedVariablesDeclaration = new List<String>(),
       buffer = new StringBuffer(),
       generateAtUseSite = new Set<HInstruction>(),
       logicalOperations = new Map<HPhi, String>(),
       breakAction = new Map<Element, ElementAction>(),
       continueAction = new Map<Element, ElementAction>(),
-      phiEquivalence = new Equivalence<HPhi>(),
       unsignedShiftPrecedences = JSPrecedence.binary['>>>'] {
 
-    for (final name in parameterNames.getValues()) {
-      prefixes[name] = 0;
-    }
-
-    // Create a namespace for temporaries.
-    prefixes[TEMPORARY_PREFIX] = 0;
-
     Interceptors interceptors = backend.builder.interceptors;
     equalsNullElement = interceptors.getEqualsNullInterceptor();
     boolifiedEqualsNullElement =
         interceptors.getBoolifiedVersionOf(equalsNullElement);
   }
 
   abstract visitTypeGuard(HTypeGuard node);
 
   abstract beginGraph(HGraph graph);
   abstract endGraph(HGraph graph);
@@ skipping 22 matching lines @@
   void endExpression(int precedence) {
     if (precedence < expectedPrecedence) {
       buffer.add(')');
     }
   }
 
   void preGenerateMethod(HGraph graph) {
     new SsaInstructionMerger(generateAtUseSite).visitGraph(graph);
     new SsaConditionMerger(generateAtUseSite,
                            logicalOperations).visitGraph(graph);
-    new PhiEquivalator(phiEquivalence, logicalOperations).analyzeGraph(graph);
+    SsaLiveIntervalBuilder intervalBuilder =

[floitsch, 2012/05/30 18:45:03]

new SsaLiveIntervalBuilder(compiler).visitGraph(graph);

[ngeoffray, 2012/05/31 08:12:46]

No, I need the object to take its liveInstructions and liveIntervals fields.

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

It's also used below.

+        new SsaLiveIntervalBuilder(compiler);
+    intervalBuilder.visitGraph(graph);
+    SsaVariableAllocator allocator = new SsaVariableAllocator(
+        compiler,
+        intervalBuilder.liveInstructions,
+        intervalBuilder.liveIntervals,
+        generateAtUseSite);
+    allocator.visitGraph(graph);
+    variableNames = allocator.names;
   }
 
   visitGraph(HGraph graph) {
     preGenerateMethod(graph);
     currentGraph = graph;
     indent++;  // We are already inside a function.
     subGraph = new SubGraph(graph.entry, graph.exit);
     beginGraph(graph);
     visitBasicBlock(graph.entry);
-    if (!delayedVarDecl.isEmpty()) {
+    if (!delayedVariablesDeclaration.isEmpty()) {
       addIndented("var ");
-      while (true) {
-        buffer.add(delayedVarDecl.head);
-        delayedVarDecl = delayedVarDecl.tail;
-        if (delayedVarDecl.isEmpty()) break;
-        buffer.add(", ");
-      }
+      buffer.add(Strings.join(delayedVariablesDeclaration, ', '));
       buffer.add(";\n");
     }
     endGraph(graph);
   }
 
   void visitSubGraph(SubGraph newSubGraph) {
     SubGraph oldSubGraph = subGraph;
     subGraph = newSubGraph;
     visitBasicBlock(subGraph.start);
     subGraph = oldSubGraph;
@@ skipping 117 matching lines @@
     int oldState = generationState;
     generationState = STATE_FIRST_DECLARATION;
     visitSubGraph(expressionSubGraph.subExpression);
     generationState = oldState;
   }
 
   void generateCondition(HBlockInformation condition) {
     generateExpression(condition);
   }
 
-  String temporary(HInstruction instruction) {
-    int id = instruction.id;
-    String name = names[id];
-    if (name !== null) return name;
-
-    String prefix = TEMPORARY_PREFIX;
-    if (instruction.sourceElement !== null) {
-      Element element = instruction.sourceElement;
-      if (element !== null && !element.name.isEmpty()) {
-        prefix = element.name.slowToString();
-        // Special case the variable named [TEMPORARY_PREFIX] to allow
-        // keeping its name.
-        if (prefix == TEMPORARY_PREFIX && !usedNames.contains(prefix)) {
-          return newName(id, prefix);
-        }
-        // If we've never seen that prefix before, try to use it
-        // directly.
-        if (!prefixes.containsKey(prefix)) {
-          // Make sure the variable name does not conflict with our mangling.
-          while (usedNames.contains(prefix)) {
-            prefix = '${prefix}_';
-          }
-          prefixes[prefix] = 0;
-          return newName(id, prefix);
-        }
-      }
-    }
-
-    name = '${prefix}${prefixes[prefix]++}';
-    while (usedNames.contains(name)) {
-      name = '${prefix}${prefixes[prefix]++}';
-    }
-    return newName(id, name);
-  }
-
-  // TODO(floitsch): share more code with [temporary].
-  String freshTemporary() {
-    String prefix = TEMPORARY_PREFIX;
-    String name = '${prefix}${prefixes[prefix]++}';
-    while (usedNames.contains(name)) {
-      name = '${prefix}${prefixes[prefix]++}';
-    }
-    String result = JsNames.getValid(name);
-    usedNames.add(result);
-    return result;
-  }
-
-  String newName(int id, String name) {
-    String result = JsNames.getValid(name);
-    names[id] = result;
-    usedNames.add(result);
-    return result;
-  }
-
   /**
     * Only visits the arguments starting at inputs[HInvoke.ARGUMENTS_OFFSET].
     */
   void visitArguments(List<HInstruction> inputs) {
     assert(inputs.length >= HInvoke.ARGUMENTS_OFFSET);
     buffer.add('(');
     for (int i = HInvoke.ARGUMENTS_OFFSET; i < inputs.length; i++) {
       if (i != HInvoke.ARGUMENTS_OFFSET) buffer.add(', ');
       use(inputs[i], JSPrecedence.ASSIGNMENT_PRECEDENCE);
     }
@@ skipping 15 matching lines @@
     return (generationState == STATE_DECLARATION ||
             generationState == STATE_FIRST_DECLARATION);
   }
 
   /**
    * Called before writing an expression.
    * Ensures that expressions are comma spearated.
    */
   void addExpressionSeparator() {
     if (generationState == STATE_FIRST_DECLARATION) {
-      buffer.add("var ");
-      generationState = STATE_DECLARATION;
+      return;

[kasperl, 2012/05/31 05:26:03]

Maybe change the structure so that you check for STATE_FIRST_EXPRESSION first
and then check for !STATE_FIRST_DECLARATION and do buffer.add there? Add a
comment that explains why you don't need an expression separator.

[ngeoffray, 2012/05/31 08:12:46]

Done.

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

Why this change?
Where do you change the state to STATE_DECLARATION?

[ngeoffray, 2012/05/31 08:48:22]

In declareVariable line 454. I need this change because the 'var' is not needed
when the variable following has already been declared.

     } else if (generationState == STATE_FIRST_EXPRESSION) {
       generationState = STATE_EXPRESSION;
     } else {
       buffer.add(", ");
     }
   }
 
+  bool isVariableDeclared(String variableName) {
+    return declaredVariables.contains(variableName);
+  }
+
   void declareVariable(String variableName) {
     if (isGeneratingExpression()) {
-      buffer.add(variableName);
-      if (!isGeneratingDeclaration()) {
-        delayedVarDecl = delayedVarDecl.prepend(variableName);
+      if (generationState == STATE_FIRST_DECLARATION) {
+        generationState = STATE_DECLARATION;
+        if (!isVariableDeclared(variableName)) {
+          declaredVariables.add(variableName);
+          buffer.add("var ");

[floitsch, 2012/05/30 18:45:03]

The 'var' is emitted conditionally. This means that we are not always in
STATE_DECLARATION.

[ngeoffray, 2012/05/31 08:12:46]

After the first declaration we must go into STATE_DECLARATION, because we cannot
introduce a 'var' anymore, and we need a comma to separate the expressions.

+        }
+      } else if (!isGeneratingDeclaration()
+                 && !isVariableDeclared(variableName)) {
+        delayedVariablesDeclaration.add(variableName);
       }
-    } else {
+    } else if (!isVariableDeclared(variableName)) {
+      declaredVariables.add(variableName);
       buffer.add("var ");
-      buffer.add(variableName);
     }
+    buffer.add(variableName);
   }
 
   void declareInstruction(HInstruction instruction) {
-    declaredInstructions.add(instruction);
-    String name = temporary(instruction);
-    declareVariable(name);
-  }
-
-  bool needsNewVariable(HInstruction instruction) {
-    bool needsVar = !instruction.usedBy.isEmpty();
-    if (needsVar && instruction is HCheck) {
-      HCheck check = instruction;
-      HInstruction input = check.checkedInput;
-      // We only need a new var if [input] is generated at use site
-      // but is not a trivial code motion invariant instruction like
-      // for parameters or this.
-      //
-      // For example:
-      // Foo a = this;
-      // print(a);
-      // print(a);
-      //
-      // In checked mode no new variable is needed.
-      // 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();
-    }
-    return needsVar;
+    declareVariable(variableNames.getName(instruction));
   }
 
   void define(HInstruction instruction) {
-    if (needsNewVariable(instruction)) {
+    if (instruction is !HCheck && variableNames.hasName(instruction)) {
       declareInstruction(instruction);
       buffer.add(" = ");
       visit(instruction, JSPrecedence.ASSIGNMENT_PRECEDENCE);
     } else {
       visit(instruction, JSPrecedence.STATEMENT_PRECEDENCE);
     }
   }
 
   void use(HInstruction argument, int expectedPrecedenceForArgument) {
-    if (argument is HCheck) {
-      HCheck instruction  = argument;
-      HInstruction input = instruction.checkedInput;
-      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)) {
-        // Get the real input of that checked instruction.
-        while (input is HCheck) {
-          HCheck check = input;
-          input = check.checkedInput;
-        }
-
-        // If [argument] cannot be generated at use site, but [input]
-        // can, use the temporary of [argument]. A code motion
-        // invariant instruction does not have a temporary, so we just
-        //
-        // For example:
-        // Foo a = foo();
-        // print(a);
-        // print(a);
-        //
-        // In checked mode will turn into:
-        // var a = FooTypeCheck(foo());
-        // print(a);
-        // print(a);
-        //
-        // Note that in case the input is code motion invariant, like
-        // for parameters or this, we just need to visit it, since
-        // there is no temporary for such instruction.
-        if (input.isCodeMotionInvariant()) {
-          visit(input, expectedPrecedenceForArgument);
-        } else {
-          buffer.add(temporary(argument));
-        }
-      } else {
-        // Otherwise we just use [input]. [argument] has already been
-        // emitted, and we just need the temporary of [input].
-        //
-        // For example:
-        // var a = foo();
-        // print(a);
-        // Foo b = a;
-        // print(b);
-        //
-        // In checked mode will turn into:
-        // var a = foo();
-        // print(a);
-        // FooTypeCheck(a);
-        // print(a);
-        use(input, expectedPrecedenceForArgument);
-      }
-    } else if (isGenerateAtUseSite(argument)) {
+    if (isGenerateAtUseSite(argument)) {
       visit(argument, expectedPrecedenceForArgument);
+    } else if (argument is HCheck) {
+      HCheck check = argument;
+      use(argument.checkedInput, expectedPrecedenceForArgument);
     } else {
-      buffer.add(temporary(argument));
+      buffer.add(variableNames.getName(argument));
     }
   }
 
   visit(HInstruction node, int expectedPrecedenceForNode) {
     int oldPrecedence = this.expectedPrecedence;
     this.expectedPrecedence = expectedPrecedenceForNode;
     node.accept(this);
     this.expectedPrecedence = oldPrecedence;
   }
 
@@ skipping 67 matching lines @@
 
   bool visitTryInfo(HTryBlockInformation info) {
     addIndented("try {\n");
     indent++;
     generateStatements(info.body);
     indent--;
     addIndented("}");
     if (info.catchBlock !== null) {
       // Printing the catch part.
       HParameterValue exception = info.catchVariable;
-      String name = temporary(exception);
+      String name = variableNames.getName(exception);
       parameterNames[exception.element] = name;
       buffer.add(' catch ($name) {\n');
       indent++;
       generateStatements(info.catchBlock);
       parameterNames.remove(exception.element);
       indent--;
       addIndented('}');
     }
     if (info.finallyBlock != null) {
       buffer.add(" finally {\n");
@@ skipping 280 matching lines @@
       return;
     }
     // Flow based traversal.
     if (node.isLoopHeader() &&
         node.loopInformation.loopBlockInformation !== currentBlockInformation) {
       beginLoop(node);
     }
     iterateBasicBlock(node);
   }
 
-  /** Generates the assignments for all phis of successors blocks. */
-  void assignPhisOfAllSuccessors(HBasicBlock node) {
-    Map<HInstruction, String> temporaryNamesOfPhis = null;
+  void emitAssignment(String source, String destination) {
+    if (isGeneratingExpression()) { 
+      addExpressionSeparator();
+    } else {
+      addIndentation();
+    }
+    declareVariable(destination);
+    buffer.add(' = $source');
+    if (!isGeneratingExpression()) {
+      buffer.add(';\n');
+    }
+  }
 
-    /**
-     * Generates the assignment [canonicalPhi] = [value].
-     *
-     * If the [canonicalPhi] has a temporary name (in [temporaryNamesOfPhis])
-     * then the temporary is assigned instead of the [canonicalPhi]. However,
-     * when the left and right-hand side are equal ([:canonicalPhi === value:])
-     * then the [canonicalPhi] is assigned with the temporary.
-     */
-    void generateAssignment(HPhi canonicalPhi, HInstruction value) {
+  /**
+   * Sequentialize a list of parallel copies.

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

Describe which criteria are being used to do the ordering/sequentializing.
"list of parallel copies" => "list of conceptually parallel copyings" (a list is
already sequential).
Change "copy" to "copying" (and pluralize accordingly). A "copy" is the result
of the act of "copying". It's the acts that you sequentialize.

[ngeoffray, 2012/05/31 08:48:22]

Added the 'conceptually'. As discussed, kept the 'copy/copies' terminology due
to (mis-)uses in the literature.

+   */
+  void sequentializeCopies(List<Copy> copies) {
+    // Map to keep track of the current location of a variable.
+    Map<String, String> currentLocation = new Map<String, String>();

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

What is a "location" here?

[ngeoffray, 2012/05/31 08:48:22]

It is a variable. It really is "the variable that holds the value of that
variable".

+
+    // Map to keep track of the initial value of a variable.
+    Map<String, String> initialValue = new Map<String, String>();
+
+    // List of variables to assign a value.
+    List<String> todo = <String>[];

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

"todo" -> "worklist".

[ngeoffray, 2012/05/31 08:48:22]

Done.

+
+    // List of variables that we can assign a value to (ie are not

[Lasse Reichstein Nielsen, 2012/05/31 08:24:56]

Describe which criteria are being used to do the ordering/sequentializing.
"list of parallel copies" => "list of conceptually parallel copyings" (a list is
already sequential).
Change "copy" to "copying" (and pluralize accordingly). A "copy" is the result
of the act of "copying". It's the acts that you sequentialize. Or are they
"assignments"?

[ngeoffray, 2012/05/31 08:48:22]

Done.

+    // being used anymore).
+    List<String> ready = <String>[];
+
+    // Prune [copies] by removing self-copies.
+    List<Copy> prunedCopies = <Copy>[];
+    for (Copy copy in copies) {
+      String sourceName = variableNames.getName(copy.source);
+      String destinationName = variableNames.getName(copy.destination);
+      if (sourceName != destinationName) {
+        prunedCopies.add(new Copy(sourceName, destinationName));
+      }
+    }
+    copies = prunedCopies;
+
+
+    // For each copy, set the current location of the source to
+    // itself, and the initial value of the destination to the source.
+    // Add the destination to the list of copies to make.
+    for (Copy copy in copies) {
+      currentLocation[copy.source] = copy.source;
+      initialValue[copy.destination] = copy.source;
+      todo.add(copy.destination);
+    }
+
+    // For each copy, if the destination does not have a current
+    // location, then we can safely assign to it.
+    for (Copy copy in copies) {
+      if (currentLocation[copy.destination] === null) {
+        ready.add(copy.destination);
+      }
+    }
+
+    while (!todo.isEmpty()) {
+      while (!ready.isEmpty()) {
+        String destination = ready.removeLast();
+        String source = initialValue[destination];
+        // Since [source] might have been updated, use the current
+        // location of [source]
+        String copy = currentLocation[source];
+        emitAssignment(copy, destination);

[floitsch, 2012/05/30 18:45:03]

I find it easier to read if the arguments are inversed:
emitAssignment(destination, copy);

[ngeoffray, 2012/05/31 08:12:46]

Done.

+        // Now [destination] is the current location of [source].
+        currentLocation[source] = destination;
+        // If [source] hasn't been updated and needs to have a value,
+        // add it to the list of variables that can be updated. Copies
+        // of [source] will now use [destination].
+        if (source == copy && initialValue[source] !== null) {
+          ready.add(source);
+        }
+      }
+
+      // Check if we have a cycle.
+      String current = todo.removeLast();
+      // If [current] is the source of a copy, and the current
+      // location of its initial value is not itself, then there is a
+      // cycle that we break by using a temporary name.
+      if (currentLocation[current] !== null

[floitsch, 2012/05/30 18:45:03]

Is this test necessary?

Clearly: current == currentLocation[initialValue[current]] means that the
assignment has succeeded.
If that's not the case then we must be in a cycle, and we shouldn't need to test
if a variable was the source of an assignment.

If I'm right maybe change the comment to:
If the assignment has been done (current ==
currentLocation[initialValue[current]]) we are done with this variable.
Otherwise we have a cycle that we need to break.
Note, that only variables that are used as source can be in cycles and we
therefore do this (cheaper) check first.

[ngeoffray, 2012/05/31 08:12:46]

Yes, it is. Because I don't remove a variable that is being 'ready' from the
todo list. Consider the example:

a = b;
c = b;

a and c will be ready and in the todo list. Once their assignments have been
done, we fetch 'c' from the todo list and the current location of the initial
value of 'c' is 'a'.
Improved the comment.

+          && current != currentLocation[initialValue[current]]) {
+        String tempName = VariableNames.SWAP_TEMP;
+        emitAssignment(current, tempName);
+        currentLocation[current] = tempName;
+        // [current] can now be safely updated. Copies of [current]
+        // will now use [tempName].
+        ready.add(current);
+      }
+    }
+  }
+
+  void assignPhisOfSuccessors(HBasicBlock node) {
+    CopyHandler handler = variableNames.getCopies(node);
+    if (handler == null) return;
+
+    sequentializeCopies(handler.copies);
+
+    for (Copy copy in handler.assignments) {
       if (isGeneratingExpression()) {
         addExpressionSeparator();
       } else {
         addIndentation();
       }
-      if (temporaryNamesOfPhis !== null &&
-          canonicalPhi !== value &&
-          temporaryNamesOfPhis.containsKey(canonicalPhi)) {
-        // This is the assignment to the temporary.
-        declareVariable(temporaryNamesOfPhis[canonicalPhi]);
-      } else if (!declaredInstructions.contains(canonicalPhi)) {
-        declareInstruction(canonicalPhi);
-      } else {
-        buffer.add(temporary(canonicalPhi));
-      }
-      buffer.add(" = ");
-      bool isLogicalOperation = logicalOperations.containsKey(canonicalPhi);
-      if (isLogicalOperation) {
-        emitLogicalOperation(canonicalPhi, logicalOperations[canonicalPhi]);
-      } else if (canonicalPhi === value) {
-        buffer.add(temporaryNamesOfPhis[value]);
-      } else {
-        use(value, JSPrecedence.ASSIGNMENT_PRECEDENCE);
-      }
+      declareVariable(variableNames.getName(copy.destination));
+      buffer.add(' = ');
+      use(copy.source, JSPrecedence.ASSIGNMENT_PRECEDENCE);
       if (!isGeneratingExpression()) {
         buffer.add(';\n');
       }
     }
-
-    // Assignments are delayed so that we don't overwrite phis that might
-    // be used as inputs.
-    // TODO(floitsch): improve phi assignments. Currently we introduce
-    // way too many temporary variables.
-    Map<HPhi, HInstruction> phiAssignments = new Map<HPhi, HInstruction>();
-
-    for (HBasicBlock successor in node.successors) {
-      int index = successor.predecessors.indexOf(node);
-      successor.forEachPhi((HPhi phi) {
-        bool isLogicalOperation = logicalOperations.containsKey(phi);
-        // In case the phi is being generated by another
-        // instruction.
-        if (isLogicalOperation && isGenerateAtUseSite(phi)) return;
-        HPhi canonicalPhi = phiEquivalence.getRepresentative(phi);
-        assert(!isLogicalOperation || canonicalPhi === phi);
-        HInstruction input = phi.inputs[index];
-        if (input is HPhi) {
-          input = phiEquivalence.getRepresentative(input);
-          // If we use the same variable, we don't need to create an
-          // assignment.
-          if (input === canonicalPhi) {
-            assert(!isLogicalOperation);
-            return;
-          }
-        }
-        phiAssignments[canonicalPhi] = input;
-      });
-    }
-
-    Set<HPhi> inputPhis = new Set<HPhi>();
-    List<HPhi> phis = <HPhi>[];
-
-    /**
-     * Transitively collects the phis that are used when emitting the [input]
-     * and adds them to [inputPhis]. Does not add phis that are equal to the
-     * [targetPhi] or are not in the [phiAssignments] map.
-     */
-    void collectInputPhis(HInstruction input, HPhi targetPhi) {
-      if (input is HPhi) {
-        HPhi canonicalPhi = phiEquivalence.getRepresentative(input);
-        // Self-updates are ok.
-        if (canonicalPhi !== targetPhi &&
-            phiAssignments.containsKey(canonicalPhi)) {
-          inputPhis.add(canonicalPhi);
-        }
-      } else if (isGenerateAtUseSite(input)) {
-        for (HInstruction inputOfInput in input.inputs) {
-          collectInputPhis(inputOfInput, targetPhi);
-        }
-      }
-    }
-
-    phiAssignments.forEach((HPhi targetPhi, HInstruction input) {
-      phis.add(targetPhi);
-      collectInputPhis(input, targetPhi);
-    });
-
-    if (inputPhis.isEmpty()) {
-      phiAssignments.forEach(generateAssignment);
-    } else {
-      // Emit the phiX=phiY assignments, taking care to break cycles.
-      // Example program:
-      //   var x = 499;
-      //   var y = 99;
-      //   while (x != 99) {
-      //     var tmp = x;  // <=== The tmp variable is removed in Ssa-form.
-      //     x = y;
-      //     y = tmp;
-      //   }
-      //
-      temporaryNamesOfPhis = new HashMap<HInstruction, String>();
-      // For phis that are used as inputs simply *always* allocate a
-      // temporary.
-      for (HPhi phi in phis) {
-        if (inputPhis.contains(phi)) {
-          String temporaryPhi = freshTemporary();
-          temporaryNamesOfPhis[phi] = temporaryPhi;
-        }
-        // The assignPhi uses temporary variables for the left-hand side if
-        // they exist.
-        generateAssignment(phi, phiAssignments[phi]);
-      }
-      // Finally assign the input phis.
-      for (HPhi phi in inputPhis) {
-        // The assignPhi special-cases phi assignments to itself and recognizes
-        // it as assignment from the temporary variable to the actual phi.
-        generateAssignment(phi, phi);
-      }
-    }
   }
 
   void iterateBasicBlock(HBasicBlock node) {
     HInstruction instruction = node.first;
     while (instruction != null) {
       if (instruction === node.last) {
-        assignPhisOfAllSuccessors(node);
+        assignPhisOfSuccessors(node);
       }
 
       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);
@@ skipping 408 matching lines @@
   visitFieldSet(HFieldSet node) {
     String name;
     if (!node.isFromActivation()) {
       name =
           compiler.namer.instanceFieldName(currentLibrary, node.name);
       beginExpression(JSPrecedence.ASSIGNMENT_PRECEDENCE);
       use(node.receiver, JSPrecedence.MEMBER_PRECEDENCE);
       buffer.add('.');
       buffer.add(name);
     } else {
-      // TODO(ngeoffray): Remove the 'var' once we don't globally box
-      // variables used in a try/catch.
-      if (!isGeneratingExpression()) {
-        buffer.add('var ');
-      }
       use(node.receiver, JSPrecedence.EXPRESSION_PRECEDENCE);
     }
     buffer.add(' = ');
     use(node.value, JSPrecedence.ASSIGNMENT_PRECEDENCE);
     if (node.receiver !== null) {
       endExpression(JSPrecedence.ASSIGNMENT_PRECEDENCE);
     }
   }
 
   visitForeign(HForeign node) {
@@ skipping 128 matching lines @@
     } else {
       beginExpression(JSPrecedence.PREFIX_PRECEDENCE);
       buffer.add('!');
       use(input, JSPrecedence.PREFIX_PRECEDENCE);
       endExpression(JSPrecedence.PREFIX_PRECEDENCE);
     }
   }
 
   visitParameterValue(HParameterValue node) {
     assert(isGenerateAtUseSite(node));
-    if (parameterNames[node.element] == null) {
-      buffer.add(temporary(node));
-    } else {
-      buffer.add(parameterNames[node.element]);
-    }
+    buffer.add(variableNames.getName(node));
   }
 
   visitPhi(HPhi node) {
     String operation = logicalOperations[node];
     if (operation !== null) {
       emitLogicalOperation(node, operation);
     } else {
-      HPhi canonicalPhi = phiEquivalence.getRepresentative(node);
-      buffer.add('${temporary(canonicalPhi)}');
+      buffer.add('${variableNames.getName(node)}');
     }
   }
 
   visitReturn(HReturn node) {
     addIndentation();
     assert(node.inputs.length == 1);
     HInstruction input = node.inputs[0];
     if (input.isConstantNull()) {
       buffer.add('return;\n');
     } else {
@@ skipping 702 matching lines @@
   void visitTypeGuard(HTypeGuard node) {
     indent--;
     addIndented('case ${node.state}:\n');
     indent++;
     addIndented('state = 0;\n');
 
     setup.add('    case ${node.state}:\n');
     int i = 0;
     for (HInstruction input in node.inputs) {
       HInstruction instruction = unwrap(input);
-      setup.add('      ${temporary(instruction)} = env$i;\n');
+      setup.add('      ${variableNames.getName(instruction)} = env$i;\n');
       i++;
     }
     if (i > maxBailoutParameters) maxBailoutParameters = i;
     setup.add('      break;\n');
   }
 
   void startBailoutCase(List<HTypeGuard> bailouts1,
                         List<HTypeGuard> bailouts2) {
     indent--;
     handleBailoutCase(bailouts1);
@@ skipping 138 matching lines @@
       startBailoutSwitch();
     }
   }
 
   void endLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
     if (labeledBlockInfo.body.start.hasGuards()) {
       endBailoutSwitch();
     }
   }
 }