Introduce a nesting stack to the flow graph builder.

Statically track nested blocks, loops, and switches while constructing
the flow graph.  Use the nesting stack as a mapping from source labels
(class SourceLabel) to their flow-graph targets (class JoinEntryInstr).

This removes the indirect dependence of the AST on the intermediate
language and the mutable compiler state from the AST.

R=fschneider@google.com, srdjan@google.com

Review URL: https://codereview.chromium.org//71703002

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@30336 260f80e4-7a28-3924-810f-c04153c831b5

runtime/vm/ast.h

@@ -833,7 +833,7 @@ class SwitchNode : public AstNode {
   }
 
   SourceLabel* label() const { return label_; }
-  AstNode* body() const { return body_; }
+  SequenceNode* body() const { return body_; }
 
   virtual void VisitChildren(AstNodeVisitor* visitor) const {
     body()->Visit(visitor);
@@ -843,7 +843,7 @@ class SwitchNode : public AstNode {
 
  private:
   SourceLabel* label_;
-  AstNode* body_;
+  SequenceNode* body_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(SwitchNode);
 };
@@ -979,9 +979,6 @@ class JumpNode : public AstNode {
       inlined_finally_list_() {
     ASSERT(label_ != NULL);
     ASSERT(kind_ == Token::kBREAK || kind_ == Token::kCONTINUE);
-    if (kind_ == Token::kCONTINUE) {
-      label_->set_is_continue_target(true);
-    }
   }
 
   SourceLabel* label() const { return label_; }

runtime/vm/flow_graph_builder.cc

@@ -42,6 +42,92 @@ DEFINE_FLAG(bool, trace_type_check_elimination, false,
 DECLARE_FLAG(bool, enable_type_checks);
 
 
+JoinEntryInstr* NestedStatement::BreakTargetFor(SourceLabel* label) {
+  if (label != label_) return NULL;
+  if (break_target_ == NULL) {
+    break_target_ =
+        new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index());
+  }
+  return break_target_;
+}
+
+
+JoinEntryInstr* NestedStatement::ContinueTargetFor(SourceLabel* label) {
+  return NULL;
+}
+
+
+// A nested statement that can be the target of a continue as well as a
+// break.
+class NestedLoop : public NestedStatement {
+ public:
+  NestedLoop(FlowGraphBuilder* owner, SourceLabel* label)
+      : NestedStatement(owner, label), continue_target_(NULL) { }
+
+  JoinEntryInstr* continue_target() const { return continue_target_; }
+
+  virtual JoinEntryInstr* ContinueTargetFor(SourceLabel* label);
+
+ private:
+  JoinEntryInstr* continue_target_;
+};
+
+
+JoinEntryInstr* NestedLoop::ContinueTargetFor(SourceLabel* label) {
+  if (label != this->label()) return NULL;
+  if (continue_target_ == NULL) {
+    continue_target_ =
+        new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index());
+  }
+  return continue_target_;
+}
+
+
+// A nested switch which can be the target of a break if labeled, and whose
+// cases can be the targets of continues.
+class NestedSwitch : public NestedStatement {
+ public:
+  NestedSwitch(FlowGraphBuilder* owner, SwitchNode* node);
+
+  virtual JoinEntryInstr* ContinueTargetFor(SourceLabel* label);
+
+ private:
+  GrowableArray<SourceLabel*> case_labels_;
+  GrowableArray<JoinEntryInstr*> case_targets_;
+};
+
+
+NestedSwitch::NestedSwitch(FlowGraphBuilder* owner, SwitchNode* node)
+    : NestedStatement(owner, node->label()),
+      case_labels_(node->body()->length()),
+      case_targets_(node->body()->length()) {
+  SequenceNode* body = node->body();
+  for (intptr_t i = 0; i < body->length(); ++i) {
+    CaseNode* case_node = body->NodeAt(i)->AsCaseNode();
+    if (case_node != NULL) {
+      case_labels_.Add(case_node->label());
+      case_targets_.Add(NULL);
+    }
+  }
+}
+
+
+JoinEntryInstr* NestedSwitch::ContinueTargetFor(SourceLabel* label) {
+  // Allocate a join for a case clause that matches the label.  This block
+  // is not necessarily targeted by a continue, but we always use a join in
+  // the graph anyway.
+  for (intptr_t i = 0; i < case_labels_.length(); ++i) {
+    if (label != case_labels_[i]) continue;
+    if (case_targets_[i] == NULL) {
+      case_targets_[i] =
+          new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index());
+    }
+    return case_targets_[i];
+  }
+  return NULL;
+}
+
+
 FlowGraphBuilder::FlowGraphBuilder(ParsedFunction* parsed_function,
                                    const Array& ic_data_array,
                                    InlineExitCollector* exit_collector,
@@ -64,6 +150,7 @@ FlowGraphBuilder::FlowGraphBuilder(ParsedFunction* parsed_function,
     graph_entry_(NULL),
     temp_count_(0),
     args_pushed_(0),
+    nesting_stack_(NULL),
     osr_id_(osr_id) { }
 
 
@@ -1567,16 +1654,14 @@ void EffectGraphVisitor::VisitIfNode(IfNode* node) {
 
 
 void EffectGraphVisitor::VisitSwitchNode(SwitchNode* node) {
+  NestedSwitch nested_switch(owner(), node);
   EffectGraphVisitor switch_body(owner());
   node->body()->Visit(&switch_body);
   Append(switch_body);
-  if ((node->label() != NULL) && (node->label()->join_for_break() != NULL)) {
-    if (is_open()) Goto(node->label()->join_for_break());
-    exit_ = node->label()->join_for_break();
+  if (nested_switch.break_target() != NULL) {
+    if (is_open()) Goto(nested_switch.break_target());
+    exit_ = nested_switch.break_target();
   }
-  // No continue label allowed.
-  ASSERT((node->label() == NULL) ||
-         (node->label()->join_for_continue() == NULL));
 }
 
 
@@ -1605,21 +1690,19 @@ void EffectGraphVisitor::VisitCaseNode(CaseNode* node) {
   const intptr_t len = node->case_expressions()->length();
   // Create case statements instructions.
   EffectGraphVisitor for_case_statements(owner());
-  // Compute start of statements fragment.
+  // Compute the start of the statements fragment.
   JoinEntryInstr* statement_start = NULL;
-  if ((node->label() != NULL) && node->label()->is_continue_target()) {
-    // Since a labeled jump continue statement occur in a different case node,
-    // allocate JoinNode here and use it as statement start.
-    statement_start = node->label()->join_for_continue();
-    if (statement_start == NULL) {
-      statement_start = new JoinEntryInstr(owner()->AllocateBlockId(),
-                                           owner()->try_index());
-      node->label()->set_join_for_continue(statement_start);
-    }
-  } else {
+  if (node->label() == NULL) {
     statement_start = new JoinEntryInstr(owner()->AllocateBlockId(),
                                          owner()->try_index());
+  } else {
+    // The case nodes are nested inside a SequenceNode that is the body of a
+    // SwitchNode.  The SwitchNode on the nesting stack contains the
+    // continue labels for all the case clauses.
+    statement_start =
+        owner()->nesting_stack()->outer()->ContinueTargetFor(node->label());
   }
+  ASSERT(statement_start != NULL);
   node->statements()->Visit(&for_case_statements);
   Instruction* statement_exit =
       AppendFragment(statement_start, for_case_statements);
@@ -1695,7 +1778,9 @@ void EffectGraphVisitor::VisitCaseNode(CaseNode* node) {
 // f) loop-exit-target
 // g) break-join (optional)
 void EffectGraphVisitor::VisitWhileNode(WhileNode* node) {
+  NestedLoop nested_loop(owner(), node->label());
   owner()->IncrementLoopDepth();
+
   TestGraphVisitor for_test(owner(), node->condition()->token_pos());
   node->condition()->Visit(&for_test);
   ASSERT(!for_test.is_empty());  // Language spec.
@@ -1704,15 +1789,13 @@ void EffectGraphVisitor::VisitWhileNode(WhileNode* node) {
   node->body()->Visit(&for_body);
 
   // Labels are set after body traversal.
-  SourceLabel* lbl = node->label();
-  ASSERT(lbl != NULL);
-  JoinEntryInstr* join = lbl->join_for_continue();
+  JoinEntryInstr* join = nested_loop.continue_target();
   if (join != NULL) {
     if (for_body.is_open()) for_body.Goto(join);
     for_body.exit_ = join;
   }
   TieLoop(node->token_pos(), for_test, for_body);
-  join = lbl->join_for_break();
+  join = nested_loop.break_target();
   if (join != NULL) {
     Goto(join);
     exit_ = join;
@@ -1730,8 +1813,10 @@ void EffectGraphVisitor::VisitWhileNode(WhileNode* node) {
 // f) loop-exit-target
 // g) break-join
 void EffectGraphVisitor::VisitDoWhileNode(DoWhileNode* node) {
+  NestedLoop nested_loop(owner(), node->label());
   owner()->IncrementLoopDepth();
-  // Traverse body first in order to generate continue and break labels.
+
+  // Traverse the body first in order to generate continue and break labels.
   EffectGraphVisitor for_body(owner());
   node->body()->Visit(&for_body);
 
@@ -1746,7 +1831,7 @@ void EffectGraphVisitor::VisitDoWhileNode(DoWhileNode* node) {
   Goto(body_entry_join);
   Instruction* body_exit = AppendFragment(body_entry_join, for_body);
 
-  JoinEntryInstr* join = node->label()->join_for_continue();
+  JoinEntryInstr* join = nested_loop.continue_target();
   if ((body_exit != NULL) || (join != NULL)) {
     if (join == NULL) {
       join = new JoinEntryInstr(owner()->AllocateBlockId(),
@@ -1762,7 +1847,7 @@ void EffectGraphVisitor::VisitDoWhileNode(DoWhileNode* node) {
   }
 
   for_test.IfTrueGoto(body_entry_join);
-  join = node->label()->join_for_break();
+  join = nested_loop.break_target();
   if (join == NULL) {
     exit_ = for_test.CreateFalseSuccessor();
   } else {
@@ -1791,6 +1876,7 @@ void EffectGraphVisitor::VisitForNode(ForNode* node) {
   Append(for_initializer);
   ASSERT(is_open());
 
+  NestedLoop nested_loop(owner(), node->label());
   owner()->IncrementLoopDepth();
   // Compose body to set any jump labels.
   EffectGraphVisitor for_body(owner());
@@ -1800,7 +1886,7 @@ void EffectGraphVisitor::VisitForNode(ForNode* node) {
   node->increment()->Visit(&for_increment);
 
   // Join the loop body and increment and then tie the loop.
-  JoinEntryInstr* continue_join = node->label()->join_for_continue();
+  JoinEntryInstr* continue_join = nested_loop.continue_target();
   if ((continue_join != NULL) || for_body.is_open()) {
     JoinEntryInstr* loop_entry =
         new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index());
@@ -1821,7 +1907,7 @@ void EffectGraphVisitor::VisitForNode(ForNode* node) {
   if (node->condition() == NULL) {
     // Endless loop, no test.
     Append(for_body);
-    exit_ = node->label()->join_for_break();  // May be NULL.
+    exit_ = nested_loop.break_target();  // May be NULL.
   } else {
     TestGraphVisitor for_test(owner(), node->condition()->token_pos());
     node->condition()->Visit(&for_test);
@@ -1830,11 +1916,11 @@ void EffectGraphVisitor::VisitForNode(ForNode* node) {
     BlockEntryInstr* body_entry = for_test.CreateTrueSuccessor();
     AppendFragment(body_entry, for_body);
 
-    if (node->label()->join_for_break() == NULL) {
+    if (nested_loop.break_target() == NULL) {
       exit_ = for_test.CreateFalseSuccessor();
     } else {
-      for_test.IfFalseGoto(node->label()->join_for_break());
-      exit_ = node->label()->join_for_break();
+      for_test.IfFalseGoto(nested_loop.break_target());
+      exit_ = nested_loop.break_target();
     }
   }
   owner()->DecrementLoopDepth();
@@ -1879,18 +1965,21 @@ void EffectGraphVisitor::VisitJumpNode(JumpNode* node) {
 
   JoinEntryInstr* jump_target = NULL;
   if (node->kind() == Token::kBREAK) {
-    if (node->label()->join_for_break() == NULL) {
-      node->label()->set_join_for_break(
-          new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index()));
+    NestedStatement* current = owner()->nesting_stack();
+    while (current != NULL) {
+      jump_target = current->BreakTargetFor(node->label());
+      if (jump_target != NULL) break;
+      current = current->outer();
     }
-    jump_target = node->label()->join_for_break();
   } else {
-    if (node->label()->join_for_continue() == NULL) {
-      node->label()->set_join_for_continue(
-          new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index()));
+    NestedStatement* current = owner()->nesting_stack();
+    while (current != NULL) {
+      jump_target = current->ContinueTargetFor(node->label());
+      if (jump_target != NULL) break;
+      current = current->outer();
     }
-    jump_target = node->label()->join_for_continue();
   }
+  ASSERT(jump_target != NULL);
   Goto(jump_target);
 }
 
@@ -3386,6 +3475,11 @@ void EffectGraphVisitor::VisitSequenceNode(SequenceNode* node) {
   const intptr_t num_context_variables =
       (scope != NULL) ? scope->num_context_variables() : 0;
   int previous_context_level = owner()->context_level();
+  // The outermost function sequence cannot contain a label.
+  ASSERT((node->label() == NULL) ||
+         (node != owner()->parsed_function()->node_sequence()));
+  NestedStatement nested_block(owner(), node->label());
+
   if (num_context_variables > 0) {
     // The loop local scope declares variables that are captured.
     // Allocate and chain a new context.
@@ -3415,6 +3509,7 @@ void EffectGraphVisitor::VisitSequenceNode(SequenceNode* node) {
       AddInstruction(
           new StoreContextInstr(Bind(ExitTempLocalScope(tmp_var))));
     }
+
     owner()->set_context_level(scope->context_level());
 
     // If this node_sequence is the body of the function being compiled, copy
@@ -3509,20 +3604,13 @@ void EffectGraphVisitor::VisitSequenceNode(SequenceNode* node) {
     }
   }
 
-  // No continue on sequence allowed.
-  ASSERT((node->label() == NULL) ||
-         (node->label()->join_for_continue() == NULL));
   // If this node sequence is labeled, a break out of the sequence will have
   // taken care of unchaining the context.
-  if ((node->label() != NULL) &&
-      (node->label()->join_for_break() != NULL)) {
-    if (is_open()) Goto(node->label()->join_for_break());
-    exit_ = node->label()->join_for_break();
+  if (nested_block.break_target() != NULL) {
+    if (is_open()) Goto(nested_block.break_target());
+    exit_ = nested_block.break_target();
   }
 
-  // The outermost function sequence cannot contain a label.
-  ASSERT((node->label() == NULL) ||
-         (node != owner()->parsed_function()->node_sequence()));
   owner()->set_context_level(previous_context_level);
 }
 

runtime/vm/flow_graph_builder.h

@@ -106,6 +106,8 @@ class InlineExitCollector: public ZoneAllocated {
 };
 
 
+class NestedStatement;
+
 // Build a flow graph from a parsed function's AST.
 class FlowGraphBuilder: public ValueObject {
  public:
@@ -170,11 +172,15 @@ class FlowGraphBuilder: public ValueObject {
   intptr_t args_pushed() const { return args_pushed_; }
   void add_args_pushed(intptr_t n) { args_pushed_ += n; }
 
+  NestedStatement* nesting_stack() const { return nesting_stack_; }
+
   // When compiling for OSR, remove blocks that are not reachable from the
   // OSR entry point.
   void PruneUnreachable();
 
  private:
+  friend class NestedStatement;  // Explicit access to nesting_stack_.
+
   intptr_t parameter_count() const {
     return num_copied_params_ + num_non_copied_params_;
   }
@@ -204,6 +210,9 @@ class FlowGraphBuilder: public ValueObject {
   // Outgoing argument stack height.
   intptr_t args_pushed_;
 
+  // A stack of enclosing nested statements.
+  NestedStatement* nesting_stack_;
+
   // The deopt id of the OSR entry or Isolate::kNoDeoptId if not compiling
   // for OSR.
   const intptr_t osr_id_;
@@ -212,6 +221,42 @@ class FlowGraphBuilder: public ValueObject {
 };
 
 
+// Base class for a stack of enclosing statements of interest (e.g.,
+// blocks (breakable) and loops (continuable)).
+class NestedStatement : public ValueObject {
+ public:
+  NestedStatement(FlowGraphBuilder* owner, const SourceLabel* label)
+      : owner_(owner),
+        label_(label),
+        outer_(owner->nesting_stack_),
+        break_target_(NULL) {
+    // Push on the owner's nesting stack.
+    owner->nesting_stack_ = this;
+  }
+
+  virtual ~NestedStatement() {
+    // Pop from the owner's nesting stack.
+    ASSERT(owner_->nesting_stack_ == this);
+    owner_->nesting_stack_ = outer_;
+  }
+
+  FlowGraphBuilder* owner() const { return owner_; }
+  const SourceLabel* label() const { return label_; }
+  NestedStatement* outer() const { return outer_; }
+  JoinEntryInstr* break_target() const { return break_target_; }
+
+  virtual JoinEntryInstr* BreakTargetFor(SourceLabel* label);
+  virtual JoinEntryInstr* ContinueTargetFor(SourceLabel* label);
+
+ private:
+  FlowGraphBuilder* owner_;
+  const SourceLabel* label_;
+  NestedStatement* outer_;
+
+  JoinEntryInstr* break_target_;
+};
+
+
 class TestGraphVisitor;
 
 // Translate an AstNode to a control-flow graph fragment for its effects

runtime/vm/flow_graph_inliner.cc

@@ -69,58 +69,6 @@ static bool IsCallRecursive(const Function& function, Definition* call) {
 }
 
 
-// TODO(zerny): Remove the ChildrenVisitor and SourceLabelResetter once we have
-// moved the label/join map for control flow out of the AST and into the flow
-// graph builder.
-
-// Default visitor to traverse child nodes.
-class ChildrenVisitor : public AstNodeVisitor {
- public:
-  ChildrenVisitor() { }
-#define DEFINE_VISIT(BaseName)                                                 \
-  virtual void Visit##BaseName##Node(BaseName##Node* node) {                   \
-    node->VisitChildren(this);                                                 \
-  }
-
-  FOR_EACH_NODE(DEFINE_VISIT);
-#undef DEFINE_VISIT
-};
-
-
-// Visitor to clear each AST node containing source labels.
-class SourceLabelResetter : public ChildrenVisitor {
- public:
-  SourceLabelResetter() { }
-  virtual void VisitSequenceNode(SequenceNode* node) {
-    Reset(node, node->label());
-  }
-  virtual void VisitCaseNode(CaseNode* node) {
-    Reset(node, node->label());
-  }
-  virtual void VisitSwitchNode(SwitchNode* node) {
-    Reset(node, node->label());
-  }
-  virtual void VisitWhileNode(WhileNode* node) {
-    Reset(node, node->label());
-  }
-  virtual void VisitDoWhileNode(DoWhileNode* node) {
-    Reset(node, node->label());
-  }
-  virtual void VisitForNode(ForNode* node) {
-    Reset(node, node->label());
-  }
-  virtual void VisitJumpNode(JumpNode* node) {
-    Reset(node, node->label());
-  }
-  void Reset(AstNode* node, SourceLabel* lbl) {
-    node->VisitChildren(this);
-    if (lbl == NULL) return;
-    lbl->join_for_break_ = NULL;
-    lbl->join_for_continue_ = NULL;
-  }
-};
-
-
 // Helper to create a parameter stub from an actual argument.
 static Definition* CreateParameterStub(intptr_t i,
                                        Value* argument,
@@ -787,8 +735,6 @@ class CallSiteInliner : public ValueObject {
       ParsedFunction* parsed_function = function_cache_[i];
       if (parsed_function->function().raw() == function.raw()) {
         *in_cache = true;
-        SourceLabelResetter reset;
-        parsed_function->node_sequence()->Visit(&reset);
         return parsed_function;
       }
     }

runtime/vm/scopes.h

@@ -16,7 +16,6 @@
 
 namespace dart {
 
-class JoinEntryInstr;
 class LocalScope;
 
 
@@ -151,10 +150,7 @@ class SourceLabel : public ZoneAllocated {
     : token_pos_(token_pos),
       name_(name),
       owner_(NULL),
-      kind_(kind),
-      join_for_break_(NULL),
-      join_for_continue_(NULL),
-      is_continue_target_(false) {
+      kind_(kind) {
   }
 
   static SourceLabel* New(intptr_t token_pos, String* name, Kind kind) {
@@ -177,45 +173,17 @@ class SourceLabel : public ZoneAllocated {
 
   Kind kind() const { return kind_; }
 
-  void set_join_for_continue(JoinEntryInstr* join) {
-    ASSERT(join_for_continue_ == NULL);
-    join_for_continue_ = join;
-  }
-
-  JoinEntryInstr* join_for_continue() const {
-    return join_for_continue_;
-  }
-
-  bool is_continue_target() const { return is_continue_target_; }
-  void set_is_continue_target(bool value) { is_continue_target_ = value; }
-
-  void set_join_for_break(JoinEntryInstr* join) {
-    ASSERT(join_for_break_ == NULL);
-    join_for_break_ = join;
-  }
-
-  JoinEntryInstr* join_for_break() const {
-    return join_for_break_;
-  }
-
   // Returns the function level of the scope in which the label is defined.
   int FunctionLevel() const;
 
   void ResolveForwardReference() { kind_ = kCase; }
 
  private:
-  // TODO(zerny): Remove this hack when the builder no longer stores state in
-  // the ast/scopes.
-  friend class SourceLabelResetter;
-
   const intptr_t token_pos_;
   const String& name_;
   LocalScope* owner_;  // Local scope declaring this label.
 
   Kind kind_;
-  JoinEntryInstr* join_for_break_;
-  JoinEntryInstr* join_for_continue_;
-  bool is_continue_target_;  // Needed for CaseNode.
 
   DISALLOW_COPY_AND_ASSIGN(SourceLabel);
 };