[turbofan] Fix control reducer for dead loops.

Note OSR special case.
Also improved robustness of OSR tests.

R=mstarzinger@chromium.org
BUG=

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

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

src/compiler/control-reducer.cc

@@ -384,7 +384,8 @@ class ControlReducerImpl {
   // Reducer implementation: perform reductions on a node.
   //===========================================================================
   Node* ReduceNode(Node* node) {
-    if (node->op()->ControlInputCount() == 1) {
+    if (node->op()->ControlInputCount() == 1 ||
+        node->opcode() == IrOpcode::kLoop) {
       // If a node has only one control input and it is dead, replace with dead.
       Node* control = NodeProperties::GetControlInput(node);
       if (control->opcode() == IrOpcode::kDead) {
@@ -611,21 +612,20 @@ void ControlReducer::TrimGraph(Zone* zone, JSGraph* jsgraph) {
 }
 
 
-Node* ControlReducer::ReducePhiForTesting(JSGraph* jsgraph,
-                                          CommonOperatorBuilder* common,
-                                          Node* node) {
+Node* ControlReducer::ReduceMerge(JSGraph* jsgraph,
+                                  CommonOperatorBuilder* common, Node* node) {
   Zone zone;
   ControlReducerImpl impl(&zone, jsgraph, common);
-  return impl.ReducePhi(node);
+  return impl.ReduceMerge(node);
 }
 
 
-Node* ControlReducer::ReduceMergeForTesting(JSGraph* jsgraph,
-                                            CommonOperatorBuilder* common,
-                                            Node* node) {
+Node* ControlReducer::ReducePhiForTesting(JSGraph* jsgraph,
+                                          CommonOperatorBuilder* common,
+                                          Node* node) {
   Zone zone;
   ControlReducerImpl impl(&zone, jsgraph, common);
-  return impl.ReduceMerge(node);
+  return impl.ReducePhi(node);
 }
 
 

src/compiler/control-reducer.h

@@ -28,15 +28,16 @@ class ControlReducer {
   // Trim nodes in the graph that are not reachable from end.
   static void TrimGraph(Zone* zone, JSGraph* graph);
 
+  // Reduces a single merge node and attached phis.
+  static Node* ReduceMerge(JSGraph* graph, CommonOperatorBuilder* builder,
+                           Node* node);
+
   // Testing interface.
   static Node* ReducePhiForTesting(JSGraph* graph,
                                    CommonOperatorBuilder* builder, Node* node);
   static Node* ReduceIfNodeForTesting(JSGraph* graph,
                                       CommonOperatorBuilder* builder,
                                       Node* node);
-  static Node* ReduceMergeForTesting(JSGraph* graph,
-                                     CommonOperatorBuilder* builder,
-                                     Node* node);
 };
 }
 }

src/compiler/osr.cc

@@ -218,6 +218,16 @@ bool OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
   // and run the control reducer to clean up the graph.
   osr_normal_entry->ReplaceUses(dead);
   osr_loop_entry->ReplaceUses(graph->start());
+
+  // Normally the control reducer removes loops whose first input is dead,
+  // but we need to avoid that because the osr_loop is reachable through
+  // the second input, so reduce it and its phis manually.
+  osr_loop->ReplaceInput(0, dead);
+  Node* node = ControlReducer::ReduceMerge(jsgraph, common, osr_loop);
+  if (node != osr_loop) osr_loop->ReplaceUses(node);
+
+  // Run the normal control reduction, which naturally trims away the dead
+  // parts of the graph.
   ControlReducer::ReduceGraph(tmp_zone, jsgraph, common);
 
   return true;

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

@@ -164,8 +164,7 @@ class ControlReducerTester : HandleAndZoneScope {
   }
 
   void ReduceMerge(Node* expect, Node* merge) {
-    Node* result =
-        ControlReducer::ReduceMergeForTesting(&jsgraph, &common, merge);
+    Node* result = ControlReducer::ReduceMerge(&jsgraph, &common, merge);
     CHECK_EQ(expect, result);
   }
 
@@ -874,8 +873,7 @@ TEST(CMergeReduce_exhaustive_4) {
       if (!selector.is_selected(i)) merge->ReplaceInput(i, R.dead);
     }
 
-    Node* result =
-        ControlReducer::ReduceMergeForTesting(&R.jsgraph, &R.common, merge);
+    Node* result = ControlReducer::ReduceMerge(&R.jsgraph, &R.common, merge);
 
     int count = selector.count;
     if (count == 0) {

test/cctest/compiler/test-osr.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "src/codegen.h"
+#include "src/compiler/all-nodes.h"
 #include "src/compiler/common-operator.h"
 #include "src/compiler/diamond.h"
 #include "src/compiler/graph.h"
@@ -112,6 +113,17 @@ class OsrDeconstructorTester : public HandleAndZoneScope {
   Node* NewOsrLoop(int num_backedges, Node* entry = NULL) {
     return NewLoop(true, num_backedges, entry);
   }
+
+  void DeconstructOsr() {
+    OsrHelper helper(0, 0);
+    helper.Deconstruct(&jsgraph, &common, main_zone());
+    AllNodes nodes(main_zone(), &graph);
+    // Should be edited out.
+    CHECK(!nodes.IsLive(osr_normal_entry));
+    CHECK(!nodes.IsLive(osr_loop_entry));
+    // No dangling nodes should be left over.
+    CHECK_EQ(0u, nodes.gray.size());
+  }
 };
 
 
@@ -122,8 +134,7 @@ TEST(Deconstruct_osr0) {
 
   T.graph.SetEnd(loop);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   CheckInputs(loop, T.start, loop);
 }
@@ -139,8 +150,7 @@ TEST(Deconstruct_osr1) {
   Node* ret = T.graph.NewNode(T.common.Return(), osr_phi, T.start, loop);
   T.graph.SetEnd(ret);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   CheckInputs(loop, T.start, loop);
   CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.ZeroConstant(), loop);
@@ -160,18 +170,18 @@ TEST(Deconstruct_osr_remove_prologue) {
   Node* ret = T.graph.NewNode(T.common.Return(), osr_phi, T.start, loop);
   T.graph.SetEnd(ret);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   CheckInputs(loop, T.start, loop);
   CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.ZeroConstant(), loop);
   CheckInputs(ret, osr_phi, T.start, loop);
 
   // The control before the loop should have been removed.
-  CHECK(d.branch->IsDead());
-  CHECK(d.if_true->IsDead());
-  CHECK(d.if_false->IsDead());
-  CHECK(d.merge->IsDead());
+  AllNodes nodes(T.main_zone(), &T.graph);
+  CHECK(!nodes.IsLive(d.branch));
+  CHECK(!nodes.IsLive(d.if_true));
+  CHECK(!nodes.IsLive(d.if_false));
+  CHECK(!nodes.IsLive(d.merge));
 }
 
 
@@ -191,8 +201,7 @@ TEST(Deconstruct_osr_with_body1) {
   Node* ret = T.graph.NewNode(T.common.Return(), osr_phi, T.start, if_false);
   T.graph.SetEnd(ret);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   CheckInputs(loop, T.start, if_true);
   CheckInputs(branch, T.p0, loop);
@@ -225,8 +234,7 @@ TEST(Deconstruct_osr_with_body2) {
   Node* ret = T.graph.NewNode(T.common.Return(), osr_phi, T.start, merge);
   T.graph.SetEnd(ret);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   CheckInputs(loop, T.start, if_true2);
   CheckInputs(branch1, T.p0, loop);
@@ -265,8 +273,7 @@ TEST(Deconstruct_osr_with_body3) {
   Node* ret = T.graph.NewNode(T.common.Return(), osr_phi, T.start, if_false2);
   T.graph.SetEnd(ret);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   CheckInputs(loop, T.start, if_false1, if_true2);
   CheckInputs(branch1, T.p0, loop);
@@ -342,8 +349,7 @@ TEST(Deconstruct_osr_nested1) {
   Node* end = T.graph.NewNode(T.common.End(), ret);
   T.graph.SetEnd(end);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   // Check structure of deconstructed graph.
   // Check inner OSR loop is directly connected to start.
@@ -410,8 +416,7 @@ TEST(Deconstruct_osr_nested2) {
   Node* end = T.graph.NewNode(T.common.End(), ret);
   T.graph.SetEnd(end);
 
-  OsrHelper helper(0, 0);
-  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+  T.DeconstructOsr();
 
   // Check structure of deconstructed graph.
   // Check inner OSR loop is directly connected to start.