Constant propagator should revisit phis when it visits predecessor block.

runtime/vm/flow_graph_optimizer.cc

 // 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.
 
 #include "vm/flow_graph_optimizer.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
@@ skipping 3645 matching lines @@
   for (intptr_t i = 0; i < defs.length(); ++i) {
     defs[i]->Accept(this);
   }
   ASSERT(ForwardInstructionIterator(block).Done());
 
   SetReachable(block->normal_entry());
 }
 
 
 void ConstantPropagator::VisitJoinEntry(JoinEntryInstr* block) {
-  ZoneGrowableArray<PhiInstr*>* phis = block->phis();
-  if (phis != NULL) {
-    for (intptr_t phi_idx = 0; phi_idx < phis->length(); ++phi_idx) {
-      PhiInstr* phi = (*phis)[phi_idx];
-      if (phi == NULL) continue;
-      phi->Accept(this);
-    }
-  }
-
+  // Phis are visited when visiting predecessor. See VisitSuccessorPhis.
   for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
     it.Current()->Accept(this);
   }
+
+  VisitSuccessorPhis(block);
 }
 
 
 void ConstantPropagator::VisitTargetEntry(TargetEntryInstr* block) {
   for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
     it.Current()->Accept(this);
   }
+
+  VisitSuccessorPhis(block);
 }
 
 
 void ConstantPropagator::VisitParallelMove(ParallelMoveInstr* instr) {
   // Parallel moves have not yet been inserted in the graph.
   UNREACHABLE();
 }
 
 
+void ConstantPropagator::VisitSuccessorPhis(BlockEntryInstr* block) {
+  // Phi value depends on the reachability of a predecessor. We have
+  // to revisit phis every time a predecessor becomes reachable.
+  GotoInstr* goto_instr = block->last_instruction()->AsGoto();
+  if (goto_instr != NULL) {
+    for (PhiIterator it(goto_instr->successor()); !it.Done(); it.Advance()) {
+      it.Current()->Accept(this);
+    }
+  }
+}
+
+
 // --------------------------------------------------------------------------
 // Analysis of control instructions.  Unconditional successors are
 // reachable.  Conditional successors are reachable depending on the
 // constant value of the condition.
 void ConstantPropagator::VisitReturn(ReturnInstr* instr) {
   // Nothing to do.
 }
 
 
 void ConstantPropagator::VisitThrow(ThrowInstr* instr) {
   // Nothing to do.
 }
 
 
 void ConstantPropagator::VisitReThrow(ReThrowInstr* instr) {
   // Nothing to do.
 }
 
 
 void ConstantPropagator::VisitGoto(GotoInstr* instr) {
   SetReachable(instr->successor());

[Kevin Millikin (Google), 2013/01/09 12:20:10]

It seems simpler to move VisitSuccessorPhis here.

 }
 
 
 void ConstantPropagator::VisitBranch(BranchInstr* instr) {
   instr->comparison()->Accept(this);
 
   // The successors may be reachable, but only if this instruction is.  (We
   // might be analyzing it because the constant value of one of its inputs
   // has changed.)
   if (reachable_->Contains(instr->GetBlock()->preorder_number())) {
@@ skipping 695 matching lines @@
 
   if (FLAG_trace_constant_propagation) {
     OS::Print("\n==== After constant propagation ====\n");
     FlowGraphPrinter printer(*graph_);
     printer.PrintBlocks();
   }
 }
 
 
 }  // namespace dart