Validate that all instructions dominate their inputs. And fix a bug where that did not happen.

lib/compiler/implementation/ssa/builder.dart

Index: lib/compiler/implementation/ssa/builder.dart
===================================================================
--- lib/compiler/implementation/ssa/builder.dart	(revision 8084)
+++ lib/compiler/implementation/ssa/builder.dart	(working copy)
@@ -437,10 +437,9 @@
   }
 
   HParameterValue getActivationParameter(Element element) {
-    if (element.isParameter()) {
-      HInstruction instruction = directLocals[element];
-      if (instruction is HParameterValue) return instruction;
-    }
+    // If the element is a parameter, we already have a

[floitsch, 2012/05/30 11:52:38]

Can you also add a comment, why we have this 'if'?
Unless I'm wrong this is an optimization to avoid redundant assignments.
Correct?

[ngeoffray, 2012/05/30 12:37:26]

There is a comment:
If the element is a parameter, we already have a HParameterValue for it.

It's just to avoid adding unnecessary new HParameterValue. So I'm not sure what
you think is missing from the comment?

[floitsch, 2012/05/30 12:59:41]

If I'm not wrong this is just to avoid wrapping parameters. Removing this line
would not change anything.
So what I would like to see (as comment): is this necessary, or just an
optimization.

[ngeoffray, 2012/05/31 08:18:33]

It is necessary because we need to share the same name between the
HParameterValue (real parameter) and the HFieldGet and HFieldSet instructions.
I'll add that as a comment.

+    // HParameterValue for it.
+    if (element.isParameter()) return directLocals[element];
 
     return builder.activationVariables.putIfAbsent(element, () {
       HParameterValue parameter = new HParameterValue(element);
@@ -545,13 +544,15 @@
 
     // Create phis for all elements in the definitions environment.
     saved.forEach((Element element, HInstruction instruction) {
-      // We know 'this' cannot be modified.
-      if (element !== closureData.thisElement) {
-        HPhi phi = new HPhi.singleInput(element, instruction);
-        loopEntry.addPhi(phi);
-        directLocals[element] = phi;
-      } else {
-        directLocals[element] = instruction;
+      if (isAccessedDirectly(element)) {
+        // We know 'this' cannot be modified.
+        if (element !== closureData.thisElement) {
+          HPhi phi = new HPhi.singleInput(element, instruction);
+          loopEntry.addPhi(phi);
+          directLocals[element] = phi;
+        } else {
+          directLocals[element] = instruction;
+        }
       }
     });
   }
@@ -3118,6 +3119,13 @@
 
   visitTryStatement(TryStatement node) {
     work.allowSpeculativeOptimization = false;
+    // Save the current locals. The catch block and the finally block
+    // must not reuse the existing locals handler. None of the variables
+    // that have been defined in the body-block will be used, but for
+    // loops we will add (unnecessary) phis that will reference the body
+    // variables. This will make it look as if the variables were used

[Lasse Reichstein Nielsen, 2012/05/30 09:35:14]

"will make" -> "makes" (prefer active voice where possible)

[ngeoffray, 2012/05/30 10:19:07]

Done.

+    // in a non-dominated block.
+    LocalsHandler savedLocals = new LocalsHandler.from(localsHandler);
     HBasicBlock enterBlock = openNewBlock();
     HTry tryInstruction = new HTry();
     List<HBasicBlock> blocks = <HBasicBlock>[];
@@ -3132,6 +3140,7 @@
     SubGraph catchGraph = null;
     HParameterValue exception = null;
     if (!node.catchBlocks.isEmpty()) {
+      localsHandler = new LocalsHandler.from(savedLocals);
       HBasicBlock block = graph.addNewBlock();
       enterBlock.addSuccessor(block);
       open(block);
@@ -3198,11 +3207,13 @@
       if (!isAborted()) blocks.add(close(new HGoto()));
 
       rethrowableException = oldRethrowableException;
+      tryInstruction.catchBlock = block;
       catchGraph = new SubGraph(block, lastOpenedBlock);
     }
 
     SubGraph finallyGraph = null;
     if (node.finallyBlock != null) {
+      localsHandler = new LocalsHandler.from(savedLocals);
       HBasicBlock finallyBlock = graph.addNewBlock();
       enterBlock.addSuccessor(finallyBlock);
       open(finallyBlock);
@@ -3218,6 +3229,9 @@
       block.addSuccessor(exitBlock);
     }
 
+    // Use the locals handler not altered by the catch and finally
+    // blocks.
+    localsHandler = savedLocals;
     open(exitBlock);
     enterBlock.setBlockFlow(
         new HTryBlockInformation(