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 8031)
+++ lib/compiler/implementation/ssa/builder.dart	(working copy)
@@ -412,8 +412,8 @@
       return lookup;
     } else {
       assert(isUsedInTry(element));
-      HParameterValue parameter = getActivationParameter(element);
-      HInstruction variable = new HFieldGet.fromActivation(parameter);
+      HInstruction instruction = getActivationValue(element);
+      HInstruction variable = new HFieldGet.fromActivation(instruction);
       builder.add(variable);
       return variable;
     }
@@ -436,11 +436,8 @@
     return res;
   }
 
-  HParameterValue getActivationParameter(Element element) {
-    if (element.isParameter()) {
-      HInstruction instruction = directLocals[element];
-      if (instruction is HParameterValue) return instruction;
-    }
+  HInstruction getActivationValue(Element element) {
+    if (element.isParameter()) return directLocals[element];

[floitsch, 2012/05/29 18:31:38]

Please add comment what is happening here.

[ngeoffray, 2012/05/30 08:31:45]

Done.

 
     return builder.activationVariables.putIfAbsent(element, () {
       HParameterValue parameter = new HParameterValue(element);
@@ -475,8 +472,8 @@
       builder.add(new HFieldSet(redirect.name, box, value));
     } else {
       assert(isUsedInTry(element));
-      HParameterValue parameter = getActivationParameter(element);
-      builder.add(new HFieldSet.fromActivation(parameter, value));
+      HInstruction instruction = getActivationValue(element);
+      builder.add(new HFieldSet.fromActivation(instruction, value));
     }
   }
 
@@ -3118,6 +3115,10 @@
 
   visitTryStatement(TryStatement node) {
     work.allowSpeculativeOptimization = false;
+    // Save the current locals. The catch block, the finally block, and
+    // the merge block need to use this state of the locals.
+    LocalsHandler savedLocals = new LocalsHandler.from(localsHandler);
+
     HBasicBlock enterBlock = openNewBlock();
     HTry tryInstruction = new HTry();
     List<HBasicBlock> blocks = <HBasicBlock>[];
@@ -3132,6 +3133,7 @@
     SubGraph catchGraph = null;
     HParameterValue exception = null;
     if (!node.catchBlocks.isEmpty()) {
+      localsHandler = new LocalsHandler.from(savedLocals);

[floitsch, 2012/05/29 18:31:38]

Add comment:
The catch block must not reuse the existing LocalsHandler. 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 in a non-dominated block.

[ngeoffray, 2012/05/30 08:31:45]

Done.

       HBasicBlock block = graph.addNewBlock();
       enterBlock.addSuccessor(block);
       open(block);
@@ -3199,10 +3201,12 @@
 
       rethrowableException = oldRethrowableException;
       catchGraph = new SubGraph(block, lastOpenedBlock);
+      tryInstruction.catchBlock = block;
     }
 
     SubGraph finallyGraph = null;
     if (node.finallyBlock != null) {
+      localsHandler = new LocalsHandler.from(savedLocals);
       HBasicBlock finallyBlock = graph.addNewBlock();
       enterBlock.addSuccessor(finallyBlock);
       open(finallyBlock);
@@ -3219,6 +3223,8 @@
     }
 
     open(exitBlock);
+    localsHandler = savedLocals;
+
     enterBlock.setBlockFlow(
         new HTryBlockInformation(
           wrapStatementGraph(bodyGraph),