Added LongJump for bailout. When trying to fix all crashes, CHECK_ALIVE did not scale well as bail…

runtime/vm/flow_graph_builder.cc

Index: runtime/vm/flow_graph_builder.cc
===================================================================
--- runtime/vm/flow_graph_builder.cc	(revision 4468)
+++ runtime/vm/flow_graph_builder.cc	(working copy)
@@ -6,12 +6,12 @@
 
 #include "vm/flags.h"
 #include "vm/intermediate_language.h"
+#include "vm/longjump.h"
 #include "vm/os.h"
 #include "vm/parser.h"
 
 namespace dart {
 
-DEFINE_FLAG(bool, trace_bailout, false, "Print bailout from graph builder.");
 DEFINE_FLAG(bool, print_flow_graph, false, "Print the IR flow graph.");
 DECLARE_FLAG(bool, enable_type_checks);
 
@@ -22,7 +22,7 @@
     entry_ = other_fragment.entry();
     exit_ = other_fragment.exit();
   } else {
-    exit()->set_successor(other_fragment.entry());
+    exit()->SetSuccessor(other_fragment.entry());
     exit_ = other_fragment.exit();
   }
 }
@@ -33,7 +33,7 @@
   if (is_empty()) {
     entry_ = exit_ = instruction;
   } else {
-    exit()->set_successor(instruction);
+    exit()->SetSuccessor(instruction);
     exit_ = instruction;
   }
 }
@@ -58,18 +58,18 @@
     Instruction* false_exit = NULL;
     TargetEntryInstr* true_entry = new TargetEntryInstr();
     *test_fragment.true_successor_address() = true_entry;
-    true_entry->set_successor(true_fragment.entry());
+    true_entry->SetSuccessor(true_fragment.entry());
     true_exit = true_fragment.is_empty() ? true_entry : true_fragment.exit();
 
     TargetEntryInstr* false_entry = new TargetEntryInstr();
     *test_fragment.false_successor_address() = false_entry;
-    false_entry->set_successor(false_fragment.entry());
+    false_entry->SetSuccessor(false_fragment.entry());
     false_exit =
         false_fragment.is_empty() ? false_entry : false_fragment.exit();
 
     exit_ = new JoinEntryInstr();
-    true_exit->set_successor(exit_);
-    false_exit->set_successor(exit_);
+    true_exit->SetSuccessor(exit_);
+    false_exit->SetSuccessor(exit_);
   }
 }
 
@@ -88,7 +88,7 @@
   if (test_fragment.can_be_true()) {
     TargetEntryInstr* body_entry = new TargetEntryInstr();
     *test_fragment.true_successor_address() = body_entry;
-    body_entry->set_successor(body_fragment.entry());
+    body_entry->SetSuccessor(body_fragment.entry());
     body_exit = body_fragment.is_empty() ? body_entry : body_fragment.exit();
   }
 
@@ -99,8 +99,8 @@
   } else {
     JoinEntryInstr* join = new JoinEntryInstr();
     AddInstruction(join);
-    join->set_successor(test_fragment.entry());
-    body_exit->set_successor(join);
+    join->SetSuccessor(test_fragment.entry());
+    body_exit->SetSuccessor(join);
   }
 
   // 3. Set the exit to the graph to be empty or a fresh target node
@@ -123,9 +123,6 @@
 
 
 void EffectGraphVisitor::Bailout(const char* reason) {
-  if (FLAG_trace_bailout) {
-    OS::Print("Flow Graph Bailout: %s\n", reason);
-  }
   owner()->Bailout(reason);
 }
 
@@ -133,7 +130,7 @@
 // 'bailout' is a statement (without a semicolon), typically a return.
 #define CHECK_ALIVE(bailout)                            \
   do {                                                  \
-    if (owner()->HasBailedOut() || !is_open()) {        \
+    if (!is_open()) {                                   \
       bailout;                                          \
     }                                                   \
   } while (false)
@@ -237,9 +234,10 @@
 //                            right: <Expression> }
 InstanceCallComp* EffectGraphVisitor::TranslateBinaryOp(
     const BinaryOpNode& node) {
+  // Operators "&&" and "||" cannot be overloaded therefore do not call
+  // operator.
   if ((node.kind() == Token::kAND) || (node.kind() == Token::kOR)) {
-    Bailout("EffectGraphVisitor::VisitBinaryOpNode");
-    return NULL;
+    Bailout("EffectGraphVisitor::VisitBinaryOpNode AND/OR");
   }
   ValueGraphVisitor for_left_value(owner(), temp_index());
   node.left()->Visit(&for_left_value);
@@ -328,14 +326,33 @@
 }
 
 
+
+InstanceCallComp* EffectGraphVisitor::TranslateUnaryOp(
+    const UnaryOpNode& node) {
+  // "!" cannot be overloaded, therefore do not call operator.
+  if (node.kind() == Token::kNOT) {
+    Bailout("EffectGraphVisitor::VisitUnaryOpNode NOT");
+  }
+  ValueGraphVisitor for_value(owner(), temp_index());
+  node.operand()->Visit(&for_value);
+  Append(for_value);
+  ZoneGrowableArray<Value*>* argument = new ZoneGrowableArray<Value*>(1);
+  argument->Add(for_value.value());
+  return new InstanceCallComp(node.Name(), argument);

[Kevin Millikin (Google), 2012/02/23 09:31:43]

We might want to make the InstanceCallComp (and other kinds of call) constructor
take varargs, or else have overloaded versions for common argument counts, and
make the array embedded in the instruction rather than indirect, and get rid of
the allocation and calls to Add at every call site.

What do you think?

[srdjan, 2012/02/23 16:06:12]

I do not think that cost of allocating ZoneGrowableArray will be relevant. We do
something similar in the old code generator and the cost of the code generator
has not shown up on the profile. I am also not in favor of using varargs but in
some very rare cases.

+}
+
+
 void EffectGraphVisitor::VisitUnaryOpNode(UnaryOpNode* node) {
-  Bailout("EffectGraphVisitor::VisitUnaryOpNode");
+  InstanceCallComp* call = TranslateUnaryOp(*node);
+  DoComputation(call);
 }
 void ValueGraphVisitor::VisitUnaryOpNode(UnaryOpNode* node) {
-  Bailout("ValueGraphVisitor::VisitUnaryOpNode");
+  InstanceCallComp* call = TranslateUnaryOp(*node);
+  ReturnValueOf(call);
 }
 void TestGraphVisitor::VisitUnaryOpNode(UnaryOpNode* node) {
-  Bailout("TestGraphVisitor::VisitUnaryOpNode");
+  InstanceCallComp* call = TranslateUnaryOp(*node);
+  BranchOnValueOf(call);
 }
 
 
@@ -856,38 +873,56 @@
 }
 
 
-void FlowGraphBuilder::TraceBailout() const {
-  if (FLAG_trace_bailout && HasBailedOut()) {
-    OS::Print("Failed: %s in %s\n",
-              bailout_reason_,
-              parsed_function().function().ToFullyQualifiedCString());
-  }
-}
-
-
 void FlowGraphBuilder::PrintGraph() const {
-  if (!FLAG_print_flow_graph || HasBailedOut()) return;
-
   OS::Print("==== %s\n",
             parsed_function().function().ToFullyQualifiedCString());
 
-  for (intptr_t i = postorder_.length() - 1; i >= 0; --i) {
-    OS::Print("%8d: ", postorder_.length() - i);
-    postorder_[i]->Print(i, postorder_);
+  for (intptr_t i = postorder_block_entries_.length() - 1; i >= 0; --i) {
+    // Print the block entry.
+    Instruction* current = postorder_block_entries_[i]->Print();
+    // And all the successors until an exit, branch, or a block entry.
+    while (current != NULL && !current->IsBlockEntry()) {
+      OS::Print("\n");
+      current = current->Print();
+    }
+    if (current != NULL && current->IsBlockEntry()) {
+      OS::Print(" goto %d", current->GetBlockNumber());
+    }
     OS::Print("\n");
   }
-  OS::Print("\n");
 }
 
 
 void FlowGraphBuilder::BuildGraph() {
   EffectGraphVisitor for_effect(this, 0);
+  for_effect.AddInstruction(new TargetEntryInstr());
   parsed_function().node_sequence()->Visit(&for_effect);
-  TraceBailout();
-  if (!HasBailedOut() && (for_effect.entry() != NULL)) {
-    for_effect.entry()->Postorder(&postorder_);
+  if (for_effect.entry() != NULL) {
+    // Accumulate basic block entries via postorder traversal.
+    for_effect.entry()->Postorder(&postorder_block_entries_);
+    // Number the blocks in reverse postorder starting with 0.
+    intptr_t index = postorder_block_entries_.length() - 1;
+    intptr_t number = 0;
+    while (index >= 0) {
+      postorder_block_entries_[index--]->SetBlockNumber(number++);
+    }
   }
-  PrintGraph();
+  if (FLAG_print_flow_graph) {
+    PrintGraph();
+  }
 }
 
+
+void FlowGraphBuilder::Bailout(const char* reason) {
+  const char* kFormat = "FlowGraphBuilder Bailout: %s";
+  intptr_t len = OS::SNPrint(NULL, 0, kFormat, reason) + 1;
+  char* chars = reinterpret_cast<char*>(
+      Isolate::Current()->current_zone()->Allocate(len));
+  OS::SNPrint(chars, len, kFormat, reason);
+  const Error& error = Error::Handle(
+      LanguageError::New(String::Handle(String::New(chars))));
+  Isolate::Current()->long_jump_base()->Jump(1, error);
+}
+
+
 }  // namespace dart