Change comparison-to-branch fusion to actually remove comparison from the graph.

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 a02a048f50c3c75d750092c4c44fcd1c185e0c58..e36936b218e892c270ec33163088974f92d19020 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -527,6 +527,8 @@ void FlowGraphOptimizer::VisitStoreIndexed(StoreIndexedComp* comp,
 }
 
 
+
+
 static void TryFuseComparisonWithBranch(BindInstr* instr,
                                         ComparisonComp* comp) {
   Instruction* next_instr = instr->next();
@@ -535,7 +537,11 @@ static void TryFuseComparisonWithBranch(BindInstr* instr,
     UseVal* use = branch->value()->AsUse();
     if (instr == use->definition()) {
       comp->MarkFusedWithBranch(branch);
-      branch->MarkFusedWithComparison();
+      branch->MarkFusedWithComparison(comp);
+
+      // Remove comparison from the graph.
+      branch->set_previous(instr->previous());
+      instr->previous()->set_next(branch);

[srdjan, 2012/07/11 16:19:38]

How about factoring this out into a Remove method? You should also clear next
and previous of comparison so that it does nto accidentally get iterated.

[Vyacheslav Egorov (Google), 2012/07/11 17:30:26]

It can't be factored into generic remove method because generic would have to
handle corner cases like removal of last instruction of the block when implicit
goto is present.  [last_instruction of the block has to be updated]. I
intentionally decided to remove it manually here because we know that it is not
the last instruction in the block.

       return;
     }
   }
@@ -549,9 +555,12 @@ static void TryFuseComparisonWithBranch(BindInstr* instr,
         if ((branch->value()->AsUse()->definition() == next_instr) &&
             (negate->value()->AsUse()->definition() == instr)) {
           comp->MarkFusedWithBranch(branch);
-          branch->MarkFusedWithComparison();
+          branch->MarkFusedWithComparison(comp);
           branch->set_is_negated(true);
-          instr->set_next(next_next_instr);
+
+          // Remove comparison and boolean negation from the graph.
+          branch->set_previous(instr->previous());
+          instr->previous()->set_next(branch);

[srdjan, 2012/07/11 16:19:38]

Ditto. Clear next/previous of branch and negate.

[Vyacheslav Egorov (Google), 2012/07/11 17:30:26]

Currently we fuse from inside visiting of comparison. Iterator will crash when
trying to access next field. Teaching it to cope with that proves difficult
(also moving fusion to visiting of the branch is a major rewrite, I just tried).

           return;
         }
       }