Fix a LICM bug where we were loop hoisting an isntruction, even though that isntruction's inputs we…

lib/compiler/implementation/ssa/optimize.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.
 
 part of ssa;
 
 abstract class OptimizationPhase {
   String get name;
   void visitGraph(HGraph graph);
 }
@@ skipping 872 matching lines @@
     moveLoopInvariantCode(graph);
     visitBasicBlock(graph.entry, new ValueSet());
   }
 
   void moveLoopInvariantCode(HGraph graph) {
     for (int i = graph.blocks.length - 1; i >= 0; i--) {
       HBasicBlock block = graph.blocks[i];
       if (block.isLoopHeader()) {
         int changesFlags = loopChangesFlags[block.id];
         HLoopInformation info = block.loopInformation;
-        HBasicBlock last = info.getLastBackEdge();
-        for (int j = block.id; j <= last.id; j++) {
-          moveLoopInvariantCodeFromBlock(graph.blocks[j], block, changesFlags);
+        // Iterate over all blocks of this loop. Note that blocks in
+        // inner loops are not visited here, but we know they
+        // were visited before because we are iterating in post-order.
+        // So instructions that are GVN'ed in an inner loop are in their
+        // loop header, and [info.blocks] contains this loop header.
+        for (HBasicBlock other in info.blocks) {
+          moveLoopInvariantCodeFromBlock(other, block, changesFlags);
         }
       }
     }
   }
 
   void moveLoopInvariantCodeFromBlock(HBasicBlock block,
                                       HBasicBlock loopHeader,
                                       int changesFlags) {
     HBasicBlock preheader = loopHeader.predecessors[0];

[kasperl, 2012/11/01 14:00:17]

Maybe assert that block is part of the loop defined by loopHeader?

[ngeoffray, 2012/11/01 16:15:19]

Done.

     int dependsFlags = HInstruction.computeDependsOnFlags(changesFlags);
     HInstruction instruction = block.first;
     while (instruction != null) {
       HInstruction next = instruction.next;
       if (instruction.useGvn()
           && (instruction is !HCheck)
           && (instruction.flags & dependsFlags) == 0) {
         bool loopInvariantInputs = true;
         List<HInstruction> inputs = instruction.inputs;
         for (int i = 0, length = inputs.length; i < length; i++) {
@@ skipping 425 matching lines @@
     }
 
     // For other fields having setters in the generative constructor body, set
     // the type to UNKNOWN to avoid relying on the type set in the initializer
     // list.
     allSetters.forEach((Element element) {
       backend.registerFieldConstructor(element, HType.UNKNOWN);
     });
   }
 }