[turbofan] Connect loops to end via Terminate during graph building.

This way we don't need to connect (potentially) non-terminating loops
later during control reduction, which saves one forward pass over the
control graph.  Long term we will move the trimming functionality of
the control reducer to the GraphReducer, and get rid of the Finish
method again.

As a bonus, this change also properly rewires Terminate, Throw and
Deoptimize during inlining.

R=mstarzinger@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#28625}

src/compiler/ast-graph-builder.cc

@@ -3660,6 +3660,14 @@ void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned,
   Node* effect = builder_->NewEffectPhi(1, GetEffectDependency(), control);
   UpdateEffectDependency(effect);
 
+  // Connect the loop to end via Terminate if it's not marked as unreachable.
+  if (!IsMarkedAsUnreachable()) {
+    // Connect the Loop node to end via a Terminate node.
+    Node* terminate = builder_->graph()->NewNode(
+        builder_->common()->Terminate(), effect, control);
+    builder_->exit_controls_.push_back(terminate);
+  }
+
   if (builder_->info()->is_osr()) {
     // Introduce phis for all context values in the case of an OSR graph.
     for (int i = 0; i < static_cast<int>(contexts()->size()); ++i) {

src/compiler/common-operator.cc

@@ -110,7 +110,7 @@ std::ostream& operator<<(std::ostream& os, ParameterInfo const& i) {
   V(Throw, Operator::kKontrol, 1, 1, 1, 0, 0, 1)           \
   V(Deoptimize, Operator::kNoThrow, 1, 1, 1, 0, 0, 1)      \
   V(Return, Operator::kNoThrow, 1, 1, 1, 0, 0, 1)          \
-  V(Terminate, Operator::kNoThrow, 0, 1, 1, 0, 0, 1)       \
+  V(Terminate, Operator::kKontrol, 0, 1, 1, 0, 0, 1)       \
   V(OsrNormalEntry, Operator::kFoldable, 0, 1, 1, 0, 1, 1) \
   V(OsrLoopEntry, Operator::kFoldable, 0, 1, 1, 0, 1, 1)
 

src/compiler/control-reducer.cc

@@ -33,18 +33,12 @@ class ReachabilityMarker : public NodeMarker<uint8_t> {
     return before & kFromEnd;
   }
   bool IsReachableFromEnd(Node* node) { return Get(node) & kFromEnd; }
-  bool SetReachableFromStart(Node* node) {
-    uint8_t before = Get(node);
-    Set(node, before | kFromStart);
-    return before & kFromStart;
-  }
-  bool IsReachableFromStart(Node* node) { return Get(node) & kFromStart; }
   void Push(Node* node) { Set(node, Get(node) | kFwStack); }
   void Pop(Node* node) { Set(node, Get(node) & ~kFwStack); }
   bool IsOnStack(Node* node) { return Get(node) & kFwStack; }
 
  private:
-  enum Bit { kFromEnd = 1, kFromStart = 2, kFwStack = 4 };
+  enum Bit { kFromEnd = 1, kFwStack = 2 };
 };
 
 
@@ -64,134 +58,11 @@ class ControlReducerImpl final : public AdvancedReducer {
   CommonOperatorBuilder* common() { return jsgraph_->common(); }
   Node* dead() { return jsgraph_->DeadControl(); }
 
-  // Finish reducing the graph by trimming nodes and/or connecting NTLs.
+  // Finish reducing the graph by trimming nodes.
   bool Finish() final {
-    bool done = true;
-    // Gather all nodes backwards-reachable from end (through inputs).
-    ReachabilityMarker marked(graph());
-    NodeVector nodes(zone_);
-    AddNodesReachableFromRoots(marked, nodes);
-
-    // Walk forward through control nodes, looking for back edges to nodes
-    // that are not connected to end. Those are non-terminating loops (NTLs).
-    Node* start = graph()->start();
-    marked.Push(start);
-    marked.SetReachableFromStart(start);
-
-    // We use a stack of (Node, Node::UseEdges::iterator) pairs to avoid
-    // O(n^2) traversal.
-    typedef std::pair<Node*, Node::UseEdges::iterator> FwIter;
-    ZoneVector<FwIter> fw_stack(zone_);
-    fw_stack.push_back(FwIter(start, start->use_edges().begin()));
-
-    while (!fw_stack.empty()) {
-      Node* node = fw_stack.back().first;
-      TRACE("ControlFw: #%d:%s\n", node->id(), node->op()->mnemonic());
-      bool pop = true;
-      while (fw_stack.back().second != node->use_edges().end()) {
-        Edge edge = *(fw_stack.back().second);
-        Node* succ = edge.from();
-        if (NodeProperties::IsControlEdge(edge) &&
-            succ->op()->ControlOutputCount() > 0) {
-          // Only walk control edges to control nodes.
-          if (marked.IsOnStack(succ) && !marked.IsReachableFromEnd(succ)) {
-            // {succ} is on stack and not reachable from end.
-            Node* added = ConnectNTL(succ);
-            nodes.push_back(added);
-            marked.SetReachableFromEnd(added);
-            AddBackwardsReachableNodes(marked, nodes, nodes.size() - 1);
-
-            // Reset the use iterators for the entire stack.
-            for (size_t i = 0; i < fw_stack.size(); i++) {
-              FwIter& iter = fw_stack[i];
-              fw_stack[i] = FwIter(iter.first, iter.first->use_edges().begin());
-            }
-            pop = false;  // restart traversing successors of this node.
-            break;
-          }
-          if (!marked.IsReachableFromStart(succ)) {
-            // {succ} is not yet reached from start.
-            marked.SetReachableFromStart(succ);
-            if (succ->opcode() != IrOpcode::kOsrLoopEntry) {
-              // Skip OsrLoopEntry; forms a confusing irredducible loop.
-              marked.Push(succ);
-              fw_stack.push_back(FwIter(succ, succ->use_edges().begin()));
-              pop = false;  // "recurse" into successor control node.
-              break;
-            }
-          }
-        }
-        ++fw_stack.back().second;
-      }
-      if (pop) {
-        marked.Pop(node);
-        fw_stack.pop_back();
-      }
-    }
-
-    // Trim references from dead nodes to live nodes first.
-    TrimNodes(marked, nodes);
-
-    // Any control nodes not reachable from start are dead, even loops.
-    for (size_t i = 0; i < nodes.size(); i++) {
-      Node* node = nodes[i];
-      if (node->op()->ControlOutputCount() > 0 &&
-          !marked.IsReachableFromStart(node) &&
-          node->opcode() != IrOpcode::kDead) {
-        TRACE("Dead: #%d:%s\n", node->id(), node->op()->mnemonic());
-        node->ReplaceUses(dead());
-        done = false;
-      }
-    }
-
-    return done;
-  }
-
-  // Connect {loop}, the header of a non-terminating loop, to the end node.
-  Node* ConnectNTL(Node* loop) {
-    TRACE("ConnectNTL: #%d:%s\n", loop->id(), loop->op()->mnemonic());
-    DCHECK_EQ(IrOpcode::kLoop, loop->opcode());
-
-    // Collect all loop effects.
-    NodeVector effects(zone_);
-    for (auto edge : loop->use_edges()) {
-      DCHECK_EQ(loop, edge.to());
-      DCHECK(NodeProperties::IsControlEdge(edge));
-      switch (edge.from()->opcode()) {
-        case IrOpcode::kPhi:
-          break;
-        case IrOpcode::kEffectPhi:
-          effects.push_back(edge.from());
-          break;
-        default:
-          break;
-      }
-    }
-
-    // Compute effects for the Return.
-    Node* effect = graph()->start();
-    int const effects_count = static_cast<int>(effects.size());
-    if (effects_count == 1) {
-      effect = effects[0];
-    } else if (effects_count > 1) {
-      effect = graph()->NewNode(common()->EffectSet(effects_count),
-                                effects_count, &effects.front());
-    }
-
-    // Add a terminate to connect the NTL to the end.
-    Node* terminate = graph()->NewNode(common()->Terminate(), effect, loop);
-
-    Node* end = graph()->end();
-    if (end->opcode() == IrOpcode::kDead) {
-      // End is actually the dead node. Make a new end.
-      end = graph()->NewNode(common()->End(1), terminate);
-      graph()->SetEnd(end);
-      return end;
-    }
-    // Append a new input to the end.
-    end->AppendInput(graph()->zone(), terminate);
-    end->set_op(common()->End(end->InputCount()));
-    return terminate;
+    // TODO(bmeurer): Move this to the GraphReducer.
+    Trim();
+    return true;
   }
 
   void AddNodesReachableFromRoots(ReachabilityMarker& marked,
@@ -366,14 +237,6 @@ class ControlReducerImpl final : public AdvancedReducer {
     if (n <= 1) return dead();            // No non-control inputs.
     if (n == 2) return node->InputAt(0);  // Only one non-control input.
 
-    // Never remove an effect phi from a (potentially non-terminating) loop.
-    // Otherwise, we might end up eliminating effect nodes, such as calls,
-    // before the loop.
-    if (node->opcode() == IrOpcode::kEffectPhi &&
-        NodeProperties::GetControlInput(node)->opcode() == IrOpcode::kLoop) {
-      return node;
-    }
-
     Node* replacement = NULL;
     auto const inputs = node->inputs();
     for (auto it = inputs.begin(); n > 1; --n, ++it) {

src/compiler/js-inlining.cc

@@ -175,8 +175,14 @@ Reduction JSInliner::InlineCall(Node* call, Node* start, Node* end) {
         effects.push_back(NodeProperties::GetEffectInput(input));
         controls.push_back(NodeProperties::GetControlInput(input));
         break;
+      case IrOpcode::kDeoptimize:
+      case IrOpcode::kTerminate:
+      case IrOpcode::kThrow:
+        jsgraph_->graph()->end()->AppendInput(jsgraph_->zone(), input);
+        jsgraph_->graph()->end()->set_op(
+            jsgraph_->common()->End(jsgraph_->graph()->end()->InputCount()));
+        break;
       default:
-        // TODO(turbofan): Handle Throw, Terminate and Deoptimize here.
         UNREACHABLE();
         break;
     }

test/cctest/compiler/test-control-reducer.cc

@@ -1261,23 +1261,6 @@ TEST(CUnusedDiamond2) {
 TEST(CDeadLoop1) {
   ControlReducerTester R;
 
-  Node* loop = R.graph.NewNode(R.common.Loop(1), R.start);
-  Branch b(R, R.p0, loop);
-  loop->ReplaceInput(0, b.if_true);  // loop is not connected to start.
-  Node* merge = R.graph.NewNode(R.common.Merge(2), R.start, b.if_false);
-  R.ReduceMergeIterative(R.start, merge);
-
-  DeadChecker dead(&R.graph);
-  dead.Check(b.if_true);
-  dead.Check(b.if_false);
-  dead.Check(b.branch);
-  dead.Check(loop);
-}
-
-
-TEST(CDeadLoop2) {
-  ControlReducerTester R;
-
   While w(R, R.p0);
   Diamond d(R, R.zero);
   // if (0) { while (p0) ; } else { }

test/unittests/compiler/common-operator-unittest.cc

@@ -55,7 +55,7 @@ const SharedOperator kSharedOperators[] = {
     SHARED(IfException, Operator::kKontrol, 0, 0, 1, 1, 0, 1),
     SHARED(Throw, Operator::kKontrol, 1, 1, 1, 0, 0, 1),
     SHARED(Return, Operator::kNoThrow, 1, 1, 1, 0, 0, 1),
-    SHARED(Terminate, Operator::kNoThrow, 0, 1, 1, 0, 0, 1)
+    SHARED(Terminate, Operator::kKontrol, 0, 1, 1, 0, 0, 1)
 #undef SHARED
 };
 

test/unittests/compiler/control-reducer-unittest.cc

@@ -53,64 +53,6 @@ class ControlReducerTest : public TypedGraphTest {
 };
 
 
-TEST_F(ControlReducerTest, NonTerminatingLoop) {
-  Node* loop = graph()->NewNode(common()->Loop(2), graph()->start());
-  loop->AppendInput(graph()->zone(), loop);
-  ReduceGraph();
-  EXPECT_THAT(graph()->end(),
-              IsEnd(graph()->start(),
-                    IsTerminate(graph()->start(),
-                                AllOf(loop, IsLoop(graph()->start(), loop)))));
-}
-
-
-TEST_F(ControlReducerTest, NonTerminatingLoopWithEffectPhi) {
-  Node* loop = graph()->NewNode(common()->Loop(2), graph()->start());
-  loop->AppendInput(graph()->zone(), loop);
-  Node* ephi = graph()->NewNode(common()->EffectPhi(2), graph()->start());
-  ephi->AppendInput(graph()->zone(), ephi);
-  ephi->AppendInput(graph()->zone(), loop);
-  ReduceGraph();
-  EXPECT_THAT(
-      graph()->end(),
-      IsEnd(graph()->start(),
-            IsTerminate(AllOf(ephi, IsEffectPhi(graph()->start(), ephi, loop)),
-                        AllOf(loop, IsLoop(graph()->start(), loop)))));
-}
-
-
-TEST_F(ControlReducerTest, NonTerminatingLoopWithTwoEffectPhis) {
-  Node* loop = graph()->NewNode(common()->Loop(2), graph()->start());
-  loop->AppendInput(graph()->zone(), loop);
-  Node* ephi1 = graph()->NewNode(common()->EffectPhi(2), graph()->start());
-  ephi1->AppendInput(graph()->zone(), ephi1);
-  ephi1->AppendInput(graph()->zone(), loop);
-  Node* ephi2 = graph()->NewNode(common()->EffectPhi(2), graph()->start());
-  ephi2->AppendInput(graph()->zone(), ephi2);
-  ephi2->AppendInput(graph()->zone(), loop);
-  ReduceGraph();
-  EXPECT_THAT(
-      graph()->end(),
-      IsEnd(graph()->start(),
-            IsTerminate(
-                IsEffectSet(
-                    AllOf(ephi1, IsEffectPhi(graph()->start(), ephi1, loop)),
-                    AllOf(ephi2, IsEffectPhi(graph()->start(), ephi2, loop))),
-                AllOf(loop, IsLoop(graph()->start(), loop)))));
-}
-
-
-TEST_F(ControlReducerTest, NonTerminatingLoopWithDeadEnd) {
-  Node* loop = graph()->NewNode(common()->Loop(2), graph()->start());
-  loop->AppendInput(graph()->zone(), loop);
-  graph()->end()->ReplaceInput(0, graph()->NewNode(common()->Dead()));
-  ReduceGraph();
-  EXPECT_THAT(graph()->end(),
-              IsEnd(IsTerminate(graph()->start(),
-                                AllOf(loop, IsLoop(graph()->start(), loop)))));
-}
-
-
 TEST_F(ControlReducerTest, PhiAsInputToBranch_true) {
   Node* p0 = Parameter(0);
   Node* branch1 = graph()->NewNode(common()->Branch(), p0, graph()->start());