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

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/flow_graph_builder.h"
 #include "vm/il_printer.h"
 #include "vm/object_store.h"
 
@@ skipping 509 matching lines @@
 
   const intptr_t class_id = ReceiverClassId(comp);
   switch (class_id) {
     case kArray:
     case kGrowableObjectArray:
       comp->set_receiver_type(static_cast<ObjectKind>(class_id));
   }
 }
 
 
+
+
 static void TryFuseComparisonWithBranch(BindInstr* instr,
                                         ComparisonComp* comp) {
   Instruction* next_instr = instr->next();
   if ((next_instr != NULL) && next_instr->IsBranch()) {
     BranchInstr* branch = next_instr->AsBranch();
     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;
     }
   }
   if ((next_instr != NULL) && next_instr->IsBind()) {
     Computation* next_comp = next_instr->AsBind()->computation();
     if (next_comp->IsBooleanNegate()) {
       Instruction* next_next_instr = next_instr->next();
       if ((next_next_instr != NULL) && next_next_instr->IsBranch()) {
         BooleanNegateComp* negate = next_comp->AsBooleanNegate();
         BranchInstr* branch = next_next_instr->AsBranch();
         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;
         }
       }
     }
   }
 }
 
 
 void FlowGraphOptimizer::VisitRelationalOp(RelationalOpComp* comp,
                                            BindInstr* instr) {
@@ skipping 53 matching lines @@
       LocationSummary* locs = it.Current()->locs();
       if ((locs != NULL) && locs->is_call()) {
         is_leaf_ = false;
         return;
       }
     }
   }
 }
 
 }  // namespace dart