Ensure that all phis inserted by load optimizer have consistent representation.

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
diff --git a/runtime/vm/flow_graph_optimizer.cc b/runtime/vm/flow_graph_optimizer.cc
index f22e386d6ceb6a8a7f31fef4b83d24f6b7ed63d0..cf7b14229490e6eb0c297d765f7e0f85af8f3928 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -390,33 +390,89 @@ void FlowGraphOptimizer::InsertConversion(Representation from,
 }
 
 
+void FlowGraphOptimizer::ConvertUse(Value* use, Representation from_rep) {
+  const Representation to_rep =
+      use->instruction()->RequiredInputRepresentation(use->use_index());
+  if (from_rep == to_rep || to_rep == kNoRepresentation) {
+    return;
+  }
+
+  Instruction* insert_before;
+  Instruction* deopt_target;
+  PhiInstr* phi = use->instruction()->AsPhi();
+  if (phi != NULL) {
+    ASSERT(phi->is_alive());
+    // For phis conversions have to be inserted in the predecessor.
+    insert_before =
+        phi->block()->PredecessorAt(use->use_index())->last_instruction();
+    deopt_target = NULL;
+  } else {
+    deopt_target = insert_before = use->instruction();
+  }
+
+  InsertConversion(from_rep, to_rep, use, insert_before, deopt_target);
+}
+
 void FlowGraphOptimizer::InsertConversionsFor(Definition* def) {
   const Representation from_rep = def->representation();
 
   for (Value::Iterator it(def->input_use_list());
        !it.Done();
        it.Advance()) {
-    Value* use = it.Current();
-    const Representation to_rep =
-        use->instruction()->RequiredInputRepresentation(use->use_index());
-    if (from_rep == to_rep || to_rep == kNoRepresentation) {
-      continue;
-    }
+    ConvertUse(it.Current(), from_rep);
+  }
+}
 
-    Instruction* insert_before;
-    Instruction* deopt_target;
-    PhiInstr* phi = use->instruction()->AsPhi();
-    if (phi != NULL) {
-      ASSERT(phi->is_alive());
-      // For phis conversions have to be inserted in the predecessor.
-      insert_before =
-          phi->block()->PredecessorAt(use->use_index())->last_instruction();
-      deopt_target = NULL;
-    } else {
-      deopt_target = insert_before = use->instruction();
+
+static bool UnboxPhi(PhiInstr* phi) {

[srdjan, 2013/06/05 15:25:21]

Add comment: return true if phi representation changed,

[Vyacheslav Egorov (Google), 2013/06/05 15:33:07]

Done.

+  Representation current = phi->representation();
+  Representation unboxed = current;
+
+  switch (phi->Type()->ToCid()) {
+    case kDoubleCid:
+      unboxed = kUnboxedDouble;
+      break;
+    case kFloat32x4Cid:
+      unboxed = kUnboxedFloat32x4;
+      break;
+    case kUint32x4Cid:
+      unboxed = kUnboxedUint32x4;
+      break;

[srdjan, 2013/06/05 15:25:21]

Do we need to handle also unboxed Mints?

[Vyacheslav Egorov (Google), 2013/06/05 15:33:07]

Simple type based unboxing unfortunately does not work for Mints: boxing double
always produces kDoubleCid instance, but boxing mint produces either smi or
kMintCid. Thus our type propagation is tells us kDynamicCid here.

We need something different to unbox mints: something that relies on
representations, not types.

+  }
+
+  if (unboxed != current) {
+    phi->set_representation(unboxed);
+    return true;
+  }
+
+  return false;
+}
+
+
+void FlowGraphOptimizer::UnboxPhis() {
+  GrowableArray<PhiInstr*> worklist(5);
+
+  // Convervatively unbox all phis that were proven to be of type Double.

[srdjan, 2013/06/05 15:25:21]

Why not Mint as well?
Maybe mention that Double means: double, Float32x4 and Uint32x4?

[Vyacheslav Egorov (Google), 2013/06/05 15:33:07]

Mint issue explained above. 

Comment fixed. 

+  for (intptr_t i = 0; i < block_order_.length(); ++i) {
+    JoinEntryInstr* join_entry = block_order_[i]->AsJoinEntry();
+    if (join_entry != NULL) {
+      for (PhiIterator it(join_entry); !it.Done(); it.Advance()) {
+        PhiInstr* phi = it.Current();
+        if (UnboxPhi(phi)) {
+          worklist.Add(phi);
+        }
+      }
     }
+  }
 
-    InsertConversion(from_rep, to_rep, use, insert_before, deopt_target);
+  while (!worklist.is_empty()) {
+    PhiInstr* phi = worklist.RemoveLast();
+    InsertConversionsFor(phi);
+
+    for (intptr_t i = 0; i < phi->InputCount(); i++) {
+      ConvertUse(phi->InputAt(i),
+                 phi->InputAt(i)->definition()->representation());
+    }
   }
 }
 
@@ -429,14 +485,7 @@ void FlowGraphOptimizer::SelectRepresentations() {
     if (join_entry != NULL) {
       for (PhiIterator it(join_entry); !it.Done(); it.Advance()) {
         PhiInstr* phi = it.Current();
-        ASSERT(phi != NULL);
-        if (phi->Type()->ToCid() == kDoubleCid) {
-          phi->set_representation(kUnboxedDouble);
-        } else if (phi->Type()->ToCid() == kFloat32x4Cid) {
-          phi->set_representation(kUnboxedFloat32x4);
-        } else if (phi->Type()->ToCid() == kUint32x4Cid) {
-          phi->set_representation(kUnboxedUint32x4);
-        }
+        UnboxPhi(phi);
       }
     }
   }