Turn definitions that do not produce results (e.g. Checks) into instructions.

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 03dffe17a25ac2b3e3b2c2d2110b8f53d73f5722..78c4c721efe30e3fec336d266d466204e443930a 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -33,22 +33,31 @@ void FlowGraphOptimizer::OptimizeComputations() {
     BlockEntryInstr* entry = block_order_[i];
     entry->Accept(this);
     for (ForwardInstructionIterator it(entry); !it.Done(); it.Advance()) {
-      Definition* defn = it.Current()->AsDefinition();
-      if (defn != NULL) {
-        Definition* result = defn->Canonicalize();
-        if (result != defn) {
-          if (result != NULL) {
-            defn->ReplaceUsesWith(result);
-            if (FLAG_trace_optimization) {
-              OS::Print("Replacing v%"Pd" with v%"Pd"\n",
-                        defn->ssa_temp_index(),
-                        result->ssa_temp_index());
-            }
-          } else if (FLAG_trace_optimization) {
-              OS::Print("Removing v%"Pd".\n", defn->ssa_temp_index());
+      Instruction* current = it.Current();
+      Instruction* replacement = current->Canonicalize();
+      if (replacement != current) {

[Florian Schneider, 2012/09/19 13:24:49]

Maybe assert that Canonicalize for Instructions returns either NULL or this.

[Vyacheslav Egorov (Google), 2012/09/21 12:51:40]

Done.

+        if ((replacement != NULL) && current->IsDefinition()) {
+          Definition* current_defn = current->AsDefinition();
+          Definition* replacement_defn = replacement->AsDefinition();
+          ASSERT(replacement_defn != NULL);
+          current_defn->ReplaceUsesWith(replacement_defn);
+          if (FLAG_trace_optimization) {
+            OS::Print("Replacing v%"Pd" with v%"Pd"\n",
+                      current_defn->ssa_temp_index(),
+                      replacement_defn->ssa_temp_index());
+          }
+        } else if (FLAG_trace_optimization) {
+          ASSERT(!current->IsDefinition() ||
+                 ((current->AsDefinition()->input_use_list() == NULL) &&
+                  (current->AsDefinition()->env_use_list() == NULL)));
+          if (current->IsDefinition()) {
+            OS::Print("Removing v%"Pd".\n",
+                      current->AsDefinition()->ssa_temp_index());
+          } else {
+            OS::Print("Removing %s\n", current->DebugName());
           }
-          it.RemoveCurrentFromGraph();
         }
+        it.RemoveCurrentFromGraph();
       }
     }
   }
@@ -414,27 +423,31 @@ bool FlowGraphOptimizer::TryReplaceWithArrayOp(InstanceCallInstr* call,
 }
 
 
-void FlowGraphOptimizer::InsertBefore(Instruction* instr,
-                                      Definition* defn,
+void FlowGraphOptimizer::InsertBefore(Instruction* next,
+                                      Instruction* instr,
                                       Environment* env,
                                       Definition::UseKind use_kind) {
-  if (env != NULL) env->DeepCopyTo(defn);
+  if (env != NULL) env->DeepCopyTo(instr);
   if (use_kind == Definition::kValue) {
-    defn->set_ssa_temp_index(flow_graph_->alloc_ssa_temp_index());
+    ASSERT(instr->IsDefinition());
+    instr->AsDefinition()->set_ssa_temp_index(
+        flow_graph_->alloc_ssa_temp_index());
   }
-  defn->InsertBefore(instr);
+  instr->InsertBefore(next);
 }
 
 
-void FlowGraphOptimizer::InsertAfter(Instruction* instr,
-                                     Definition* defn,
+void FlowGraphOptimizer::InsertAfter(Instruction* prev,
+                                     Instruction* instr,
                                      Environment* env,
                                      Definition::UseKind use_kind) {
-  if (env != NULL) env->DeepCopyTo(defn);
+  if (env != NULL) env->DeepCopyTo(instr);
   if (use_kind == Definition::kValue) {
-    defn->set_ssa_temp_index(flow_graph_->alloc_ssa_temp_index());
+    ASSERT(instr->IsDefinition());
+    instr->AsDefinition()->set_ssa_temp_index(
+        flow_graph_->alloc_ssa_temp_index());
   }
-  defn->InsertAfter(instr);
+  instr->InsertAfter(prev);
 }
 
 
@@ -1431,7 +1444,7 @@ static BlockEntryInstr* FindPreHeader(BlockEntryInstr* header) {
 
 void LICM::Hoist(ForwardInstructionIterator* it,
                  BlockEntryInstr* pre_header,
-                 Definition* current) {
+                 Instruction* current) {
   // TODO(fschneider): Avoid repeated deoptimization when
   // speculatively hoisting checks.
   if (FLAG_trace_optimization) {
@@ -1456,7 +1469,7 @@ void LICM::Hoist(ForwardInstructionIterator* it,
 void LICM::TryHoistCheckSmiThroughPhi(ForwardInstructionIterator* it,
                                       BlockEntryInstr* header,
                                       BlockEntryInstr* pre_header,
-                                      Definition* current) {
+                                      Instruction* current) {
   PhiInstr* phi = current->InputAt(0)->definition()->AsPhi();
   if (!header->loop_info()->Contains(phi->block()->preorder_number())) {
     return;
@@ -1513,10 +1526,8 @@ void LICM::Optimize(FlowGraph* flow_graph) {
       for (ForwardInstructionIterator it(block);
            !it.Done();
            it.Advance()) {
-        Definition* current = it.Current()->AsDefinition();
-        if (current != NULL &&
-            !current->IsPushArgument() &&
-            !current->AffectedBySideEffect()) {
+        Instruction* current = it.Current();
+        if (!current->IsPushArgument() && !current->AffectedBySideEffect()) {
           bool inputs_loop_invariant = true;
           for (int i = 0; i < current->InputCount(); ++i) {
             Definition* input_def = current->InputAt(i)->definition();
@@ -1736,30 +1747,33 @@ void DominatorBasedCSE::Optimize(FlowGraph* graph) {
     }
   }
 
-  DirectChainedHashMap<Definition*> map;
+  DirectChainedHashMap<Instruction*> map;
   OptimizeRecursive(graph->graph_entry(), &map);
 }
 
 
 void DominatorBasedCSE::OptimizeRecursive(
     BlockEntryInstr* block,
-    DirectChainedHashMap<Definition*>* map) {
+    DirectChainedHashMap<Instruction*>* map) {
   for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
-    Definition* defn = it.Current()->AsDefinition();
-    if ((defn == NULL) || defn->AffectedBySideEffect()) continue;
-    Definition* result = map->Lookup(defn);
-    if (result == NULL) {
-      map->Insert(defn);
+    Instruction* current = it.Current();
+    if (current->AffectedBySideEffect()) continue;
+    Instruction* replacement = map->Lookup(current);
+    if (replacement == NULL) {
+      map->Insert(current);
       continue;
     }
     // Replace current with lookup result.
-    defn->ReplaceUsesWith(result);
-    it.RemoveCurrentFromGraph();
-    if (FLAG_trace_optimization) {
-      OS::Print("Replacing v%"Pd" with v%"Pd"\n",
-                defn->ssa_temp_index(),
-                result->ssa_temp_index());
+    if (current->IsDefinition()) {
+      ASSERT(replacement->IsDefinition());
+      current->AsDefinition()->ReplaceUsesWith(replacement->AsDefinition());
+      if (FLAG_trace_optimization) {
+        OS::Print("Replacing v%"Pd" with v%"Pd"\n",
+                  current->AsDefinition()->ssa_temp_index(),
+                  replacement->AsDefinition()->ssa_temp_index());
+      }
     }

[Florian Schneider, 2012/09/19 13:24:49]

Maybe add the same debug output as in the Canonicalize-pass:

OS::Print("Removing %s\n", current->DebugName());

[Vyacheslav Egorov (Google), 2012/09/21 12:51:40]

Done.

+    it.RemoveCurrentFromGraph();
   }
 
   // Process children in the dominator tree recursively.
@@ -1767,7 +1781,7 @@ void DominatorBasedCSE::OptimizeRecursive(
   for (intptr_t i = 0; i < num_children; ++i) {
     BlockEntryInstr* child = block->dominated_blocks()[i];
     if (i  < num_children - 1) {
-      DirectChainedHashMap<Definition*> child_map(*map);  // Copy map.
+      DirectChainedHashMap<Instruction*> child_map(*map);  // Copy map.
       OptimizeRecursive(child, &child_map);
     } else {
       OptimizeRecursive(child, map);  // Reuse map for the last child.