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

runtime/vm/flow_graph_optimizer.cc

 // Copyright (c) 2013, 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 372 matching lines @@
   } else if ((from == kUnboxedUint32x4) && (to == kTagged)) {
     converted = new BoxUint32x4Instr(use->CopyWithType());
   }
   ASSERT(converted != NULL);
   use->BindTo(converted);
   InsertBefore(insert_before, converted, use->instruction()->env(),
                Definition::kValue);
 }
 
 
+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;
-    }
-
-    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);
+    ConvertUse(it.Current(), from_rep);
   }
 }
 
+
+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);
+        }
+      }
+    }
+  }
+
+  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());
+    }
+  }
+}
+
 
 void FlowGraphOptimizer::SelectRepresentations() {
   // Convervatively unbox all phis that were proven to be of Double,
   // Float32x4, or Uint32x4.
   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();
-        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);
       }
     }
   }
 
   // Process all instructions and insert conversions where needed.
   GraphEntryInstr* graph_entry = block_order_[0]->AsGraphEntry();
 
   // Visit incoming parameters and constants.
   for (intptr_t i = 0; i < graph_entry->initial_definitions()->length(); i++) {
     InsertConversionsFor((*graph_entry->initial_definitions())[i]);
@@ skipping 6097 matching lines @@
 
   // Insert materializations at environment uses.
   const Class& cls = Class::Handle(alloc->constructor().Owner());
   for (intptr_t i = 0; i < exits.length(); i++) {
     CreateMaterializationAt(exits[i], alloc, cls, *fields);
   }
 }
 
 
 }  // namespace dart