Get rid of CHECK_ALIVE, test only is_open during sequence node traversal.

runtime/vm/flow_graph_builder.cc

Index: runtime/vm/flow_graph_builder.cc
===================================================================
--- runtime/vm/flow_graph_builder.cc	(revision 4757)
+++ runtime/vm/flow_graph_builder.cc	(working copy)
@@ -134,28 +134,17 @@
 }
 
 
-// 'bailout' is a statement (without a semicolon), typically a return.
-#define CHECK_ALIVE(bailout)                            \
-  do {                                                  \
-    if (!is_open()) {                                   \
-      bailout;                                          \
-    }                                                   \
-  } while (false)
-
-
 // <Statement> ::= Return { value:                <Expression>
 //                          inlined_finally_list: <InlinedFinally>* }
 void EffectGraphVisitor::VisitReturnNode(ReturnNode* node) {
   ValueGraphVisitor for_value(owner(), temp_index());
   node->value()->Visit(&for_value);
   Append(for_value);
-  CHECK_ALIVE(return);
 
   for (intptr_t i = 0; i < node->inlined_finally_list_length(); i++) {
     EffectGraphVisitor for_effect(owner(), for_value.temp_index());
     node->InlinedFinallyNodeAt(i)->Visit(&for_effect);
     Append(for_effect);

[Kevin Millikin (Google), 2012/03/01 09:13:53]

I think we also need

if (!is_open()) return;

immediately after this call to append.  I can't construct an example because we
bail out on TryCatchNode.

I think the rule is: before connecting a successor to the graph fragment of an
EffectGraphVisitor, check and handle !is_open().

[srdjan, 2012/03/01 16:54:58]

Done.

-    CHECK_ALIVE(return);
   }
 
   Value* return_value = for_value.value();
@@ -211,27 +200,23 @@
   ValueGraphVisitor for_value(owner(), temp_index());
   node.expr()->Visit(&for_value);
   Append(for_value);
-  CHECK_ALIVE(return NULL);
 
   return new AssertAssignableComp(for_value.value(), node.type());
 }
 
 void EffectGraphVisitor::VisitAssignableNode(AssignableNode* node) {
   AssertAssignableComp* assert = TranslateAssignable(*node);
-  CHECK_ALIVE(return);
   DoComputation(assert);
 }
 
 void ValueGraphVisitor::VisitAssignableNode(AssignableNode* node) {
   AssertAssignableComp* assert = TranslateAssignable(*node);
-  CHECK_ALIVE(return);
   ReturnValueOf(assert);
 }
 
 
 void TestGraphVisitor::VisitAssignableNode(AssignableNode* node) {
   AssertAssignableComp* assert = TranslateAssignable(*node);
-  CHECK_ALIVE(return);
   BranchOnValueOf(assert);
 }
 
@@ -249,11 +234,9 @@
   ValueGraphVisitor for_left_value(owner(), temp_index());
   node.left()->Visit(&for_left_value);
   Append(for_left_value);
-  CHECK_ALIVE(return NULL);
   ValueGraphVisitor for_right_value(owner(), for_left_value.temp_index());
   node.right()->Visit(&for_right_value);
   Append(for_right_value);
-  CHECK_ALIVE(return NULL);
   ZoneGrowableArray<Value*>* arguments = new ZoneGrowableArray<Value*>(2);
   arguments->Add(for_left_value.value());
   arguments->Add(for_right_value.value());
@@ -262,19 +245,16 @@
 
 void EffectGraphVisitor::VisitBinaryOpNode(BinaryOpNode* node) {
   InstanceCallComp* call = TranslateBinaryOp(*node);
-  CHECK_ALIVE(return);
   DoComputation(call);
 }
 
 void ValueGraphVisitor::VisitBinaryOpNode(BinaryOpNode* node) {
   InstanceCallComp* call = TranslateBinaryOp(*node);
-  CHECK_ALIVE(return);
   ReturnValueOf(call);
 }
 
 void TestGraphVisitor::VisitBinaryOpNode(BinaryOpNode* node) {
   InstanceCallComp* call = TranslateBinaryOp(*node);
-  CHECK_ALIVE(return);
   BranchOnValueOf(call);
 }
 
@@ -303,11 +283,9 @@
   ValueGraphVisitor for_left_value(owner(), temp_index());
   node.left()->Visit(&for_left_value);
   Append(for_left_value);
-  CHECK_ALIVE(return NULL);
   ValueGraphVisitor for_right_value(owner(), for_left_value.temp_index());
   node.right()->Visit(&for_right_value);
   Append(for_right_value);
-  CHECK_ALIVE(return NULL);
   if ((node.kind() == Token::kEQ_STRICT) ||
       (node.kind() == Token::kNE_STRICT)) {
     return new StrictCompareComp(
@@ -321,19 +299,16 @@
 
 void EffectGraphVisitor::VisitComparisonNode(ComparisonNode* node) {
   Computation* call = TranslateComparison(*node);
-  CHECK_ALIVE(return);
   DoComputation(call);
 }
 
 void ValueGraphVisitor::VisitComparisonNode(ComparisonNode* node) {
   Computation* call = TranslateComparison(*node);
-  CHECK_ALIVE(return);
   ReturnValueOf(call);
 }
 
 void TestGraphVisitor::VisitComparisonNode(ComparisonNode* node) {
   Computation* call = TranslateComparison(*node);
-  CHECK_ALIVE(return);
   BranchOnValueOf(call);
 }
 
@@ -563,14 +538,12 @@
   ValueGraphVisitor for_receiver(owner(), temp_index());
   node.receiver()->Visit(&for_receiver);
   Append(for_receiver);
-  CHECK_ALIVE(return NULL);
   values->Add(for_receiver.value());
   int index = temp_index();
   for (intptr_t i = 0; i < length; ++i) {
     ValueGraphVisitor for_value(owner(), index);
     arguments->NodeAt(i)->Visit(&for_value);
     Append(for_value);
-    CHECK_ALIVE(return NULL);
     values->Add(for_value.value());
     index = for_value.temp_index();
   }
@@ -580,17 +553,14 @@
 
 void EffectGraphVisitor::VisitInstanceCallNode(InstanceCallNode* node) {
   InstanceCallComp* call = TranslateInstanceCall(*node);
-  CHECK_ALIVE(return);
   DoComputation(call);
 }
 void ValueGraphVisitor::VisitInstanceCallNode(InstanceCallNode* node) {
   InstanceCallComp* call = TranslateInstanceCall(*node);
-  CHECK_ALIVE(return);
   ReturnValueOf(call);
 }
 void TestGraphVisitor::VisitInstanceCallNode(InstanceCallNode* node) {
   InstanceCallComp* call = TranslateInstanceCall(*node);
-  CHECK_ALIVE(return);
   BranchOnValueOf(call);
 }
 
@@ -602,7 +572,6 @@
     ValueGraphVisitor for_value(owner(), index);
     node.NodeAt(i)->Visit(&for_value);
     Append(for_value);
-    CHECK_ALIVE(return);
     Value* argument_value = for_value.value();
     index = for_value.temp_index();
     if (argument_value->IsConstant()) {
@@ -620,25 +589,21 @@
   int length = node.arguments()->length();
   ZoneGrowableArray<Value*>* values = new ZoneGrowableArray<Value*>(length);
   TranslateArgumentList(*node.arguments(), values);
-  CHECK_ALIVE(return NULL);
   return new StaticCallComp(node.function(), values);
 }
 
 void EffectGraphVisitor::VisitStaticCallNode(StaticCallNode* node) {
   StaticCallComp* call = TranslateStaticCall(*node);
-  CHECK_ALIVE(return);
   DoComputation(call);
 }
 
 void ValueGraphVisitor::VisitStaticCallNode(StaticCallNode* node) {
   StaticCallComp* call = TranslateStaticCall(*node);
-  CHECK_ALIVE(return);
   ReturnValueOf(call);
 }
 
 void TestGraphVisitor::VisitStaticCallNode(StaticCallNode* node) {
   StaticCallComp* call = TranslateStaticCall(*node);
-  CHECK_ALIVE(return);
   BranchOnValueOf(call);
 }
 
@@ -765,25 +730,21 @@
   ValueGraphVisitor for_value(owner(), temp_index());
   node.value()->Visit(&for_value);
   Append(for_value);
-  CHECK_ALIVE(return NULL);
   return new StoreLocalComp(node.local(), for_value.value());
 }
 
 void EffectGraphVisitor::VisitStoreLocalNode(StoreLocalNode* node) {
   StoreLocalComp* store = TranslateStoreLocal(*node);
-  CHECK_ALIVE(return);
   DoComputation(store);
 }
 
 void ValueGraphVisitor::VisitStoreLocalNode(StoreLocalNode* node) {
   StoreLocalComp* store = TranslateStoreLocal(*node);
-  CHECK_ALIVE(return);
   ReturnValueOf(store);
 }
 
 void TestGraphVisitor::VisitStoreLocalNode(StoreLocalNode* node) {
   StoreLocalComp* store = TranslateStoreLocal(*node);
-  CHECK_ALIVE(return);
   BranchOnValueOf(store);
 }
 
@@ -867,11 +828,11 @@
       (node->scope()->num_context_variables() != 0)) {
     Bailout("Sequence needs a context.  Gotta have a context.");
   }
-  for (intptr_t i = 0; i < node->length(); ++i) {
+  intptr_t i = 0;
+  while (is_open() && (i < node->length())) {
     EffectGraphVisitor for_effect(owner(), temp_index());
-    node->NodeAt(i)->Visit(&for_effect);
+    node->NodeAt(i++)->Visit(&for_effect);
     Append(for_effect);
-    CHECK_ALIVE(return);
   }
 }