[turbofan] Run DeadCodeElimination together with the advanced reducers.

This will immediately remove dead code from the graph once any of
the advanced reducers inserts it. Also changes the GraphReducer to
use the canonical Dead node for ReplaceWithValue.

R=jarin@chromium.org

Committed: https://crrev.com/88a40c5fb381924b1c0b2403dc582bceb2abe5da
Cr-Commit-Position: refs/heads/master@{#29217}

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

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

src/compiler/common-operator-reducer.cc

@@ -68,21 +68,20 @@ Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
   Decision const decision = DecideCondition(cond);
   if (decision == Decision::kUnknown) return NoChange();
   Node* const control = node->InputAt(1);
-  node->set_op(common()->Dead());
-  node->TrimInputCount(0);
+  if (!dead_.is_set()) dead_.set(graph()->NewNode(common()->Dead()));
   for (Node* const use : node->uses()) {
     switch (use->opcode()) {
       case IrOpcode::kIfTrue:
-        Replace(use, (decision == Decision::kTrue) ? control : node);
+        Replace(use, (decision == Decision::kTrue) ? control : dead_.get());
         break;
       case IrOpcode::kIfFalse:
-        Replace(use, (decision == Decision::kFalse) ? control : node);
+        Replace(use, (decision == Decision::kFalse) ? control : dead_.get());
         break;
       default:
         UNREACHABLE();
     }
   }
-  return Changed(node);
+  return Replace(dead_.get());
 }
 
 

src/compiler/common-operator-reducer.h

@@ -49,6 +49,7 @@ class CommonOperatorReducer final : public AdvancedReducer {
   Graph* const graph_;
   CommonOperatorBuilder* const common_;
   MachineOperatorBuilder* const machine_;
+  SetOncePointer<Node> dead_;
 };
 
 }  // namespace compiler

src/compiler/graph-reducer.cc

@@ -22,11 +22,9 @@ enum class GraphReducer::State : uint8_t {
 };
 
 
-GraphReducer::GraphReducer(Zone* zone, Graph* graph, Node* dead_value,
-                           Node* dead_control)
+GraphReducer::GraphReducer(Zone* zone, Graph* graph, Node* dead)
     : graph_(graph),
-      dead_value_(dead_value),
-      dead_control_(dead_control),
+      dead_(dead),
       state_(graph, 4),
       reducers_(zone),
       revisit_(zone),
@@ -205,17 +203,15 @@ void GraphReducer::ReplaceWithValue(Node* node, Node* value, Node* effect,
 
   // Requires distinguishing between value, effect and control edges.
   for (Edge edge : node->use_edges()) {
-    Node* user = edge.from();
+    Node* const user = edge.from();
+    DCHECK(!user->IsDead());
     if (NodeProperties::IsControlEdge(edge)) {
       if (user->opcode() == IrOpcode::kIfSuccess) {
         Replace(user, control);
       } else if (user->opcode() == IrOpcode::kIfException) {
-        for (Edge e : user->use_edges()) {
-          if (NodeProperties::IsValueEdge(e)) e.UpdateTo(dead_value_);
-          if (NodeProperties::IsEffectEdge(e)) e.UpdateTo(graph()->start());
-          if (NodeProperties::IsControlEdge(e)) e.UpdateTo(dead_control_);
-        }
-        edge.UpdateTo(user);
+        DCHECK_NOT_NULL(dead_);
+        edge.UpdateTo(dead_);
+        Revisit(user);
       } else {
         UNREACHABLE();
       }

src/compiler/graph-reducer.h

@@ -116,8 +116,7 @@ class AdvancedReducer : public Reducer {
 // Performs an iterative reduction of a node graph.
 class GraphReducer : public AdvancedReducer::Editor {
  public:
-  GraphReducer(Zone* zone, Graph* graph, Node* dead_value = nullptr,
-               Node* dead_control = nullptr);
+  GraphReducer(Zone* zone, Graph* graph, Node* dead = nullptr);
   ~GraphReducer();
 
   Graph* graph() const { return graph_; }
@@ -164,8 +163,7 @@ class GraphReducer : public AdvancedReducer::Editor {
   void Revisit(Node* node) final;
 
   Graph* const graph_;
-  Node* dead_value_;
-  Node* dead_control_;
+  Node* const dead_;
   NodeMarker<State> state_;
   ZoneVector<Reducer*> reducers_;
   ZoneStack<Node*> revisit_;

src/compiler/js-intrinsic-lowering.cc

@@ -132,31 +132,18 @@ Reduction JSIntrinsicLowering::ReduceDateField(Node* node) {
 
 Reduction JSIntrinsicLowering::ReduceDeoptimizeNow(Node* node) {
   if (mode() != kDeoptimizationEnabled) return NoChange();
-  Node* frame_state = NodeProperties::GetFrameStateInput(node, 0);
-  DCHECK_EQ(frame_state->opcode(), IrOpcode::kFrameState);
-
-  Node* effect = NodeProperties::GetEffectInput(node);
-  Node* control = NodeProperties::GetControlInput(node);
-
-  // We are making the continuation after the call dead. To
-  // model this, we generate if (true) statement with deopt
-  // in the true branch and continuation in the false branch.
-  Node* branch =
-      graph()->NewNode(common()->Branch(), jsgraph()->TrueConstant(), control);
-
-  // False branch - the original continuation.
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  ReplaceWithValue(node, jsgraph()->UndefinedConstant(), effect, if_false);
-
-  // True branch: deopt.
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-  Node* deopt =
-      graph()->NewNode(common()->Deoptimize(), frame_state, effect, if_true);
+  Node* const frame_state = NodeProperties::GetFrameStateInput(node, 0);
+  Node* const effect = NodeProperties::GetEffectInput(node);
+  Node* const control = NodeProperties::GetControlInput(node);
 
-  // Connect the deopt to the merge exiting the graph.
-  NodeProperties::MergeControlToEnd(graph(), common(), deopt);
+  // TODO(bmeurer): Move MergeControlToEnd() to the AdvancedReducer.
+  Node* deoptimize =
+      graph()->NewNode(common()->Deoptimize(), frame_state, effect, control);
+  NodeProperties::MergeControlToEnd(graph(), common(), deoptimize);
 
-  return Changed(deopt);
+  node->set_op(common()->Dead());
+  node->TrimInputCount(0);
+  return Changed(node);
 }
 
 

src/compiler/pipeline.cc

@@ -407,8 +407,7 @@ class SourcePositionWrapper final : public Reducer {
 class JSGraphReducer final : public GraphReducer {
  public:
   JSGraphReducer(JSGraph* jsgraph, Zone* zone)
-      : GraphReducer(zone, jsgraph->graph(), jsgraph->TheHoleConstant(),
-                     jsgraph->Dead()) {}
+      : GraphReducer(zone, jsgraph->graph(), jsgraph->Dead()) {}
   ~JSGraphReducer() final {}
 };
 
@@ -565,6 +564,8 @@ struct TypedLoweringPhase {
 
   void Run(PipelineData* data, Zone* temp_zone) {
     JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
+    DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(),
+                                              data->common());
     LoadElimination load_elimination(&graph_reducer);
     JSBuiltinReducer builtin_reducer(&graph_reducer, data->jsgraph());
     JSTypedLowering typed_lowering(&graph_reducer, data->jsgraph(), temp_zone);
@@ -575,6 +576,7 @@ struct TypedLoweringPhase {
             : JSIntrinsicLowering::kDeoptimizationDisabled);
     CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
                                          data->common(), data->machine());
+    AddReducer(data, &graph_reducer, &dead_code_elimination);
     AddReducer(data, &graph_reducer, &builtin_reducer);
     AddReducer(data, &graph_reducer, &typed_lowering);
     AddReducer(data, &graph_reducer, &intrinsic_lowering);
@@ -593,10 +595,13 @@ struct SimplifiedLoweringPhase {
                                 data->source_positions());
     lowering.LowerAllNodes();
     JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
+    DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(),
+                                              data->common());
     ValueNumberingReducer vn_reducer(temp_zone);
     MachineOperatorReducer machine_reducer(data->jsgraph());
     CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
                                          data->common(), data->machine());
+    AddReducer(data, &graph_reducer, &dead_code_elimination);
     AddReducer(data, &graph_reducer, &vn_reducer);
     AddReducer(data, &graph_reducer, &machine_reducer);
     AddReducer(data, &graph_reducer, &common_reducer);
@@ -621,11 +626,14 @@ struct ChangeLoweringPhase {
 
   void Run(PipelineData* data, Zone* temp_zone) {
     JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
+    DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(),
+                                              data->common());
     ValueNumberingReducer vn_reducer(temp_zone);
     ChangeLowering lowering(data->jsgraph());
     MachineOperatorReducer machine_reducer(data->jsgraph());
     CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
                                          data->common(), data->machine());
+    AddReducer(data, &graph_reducer, &dead_code_elimination);
     AddReducer(data, &graph_reducer, &vn_reducer);
     AddReducer(data, &graph_reducer, &lowering);
     AddReducer(data, &graph_reducer, &machine_reducer);
@@ -635,20 +643,6 @@ struct ChangeLoweringPhase {
 };
 
 
-struct LateControlReductionPhase {
-  static const char* phase_name() { return "late control reduction"; }
-  void Run(PipelineData* data, Zone* temp_zone) {
-    GraphReducer graph_reducer(temp_zone, data->graph());
-    DeadCodeElimination dce(&graph_reducer, data->graph(), data->common());
-    CommonOperatorReducer common(&graph_reducer, data->graph(), data->common(),
-                                 data->machine());
-    graph_reducer.AddReducer(&dce);
-    graph_reducer.AddReducer(&common);
-    graph_reducer.ReduceGraph();
-  }
-};
-
-
 struct EarlyGraphTrimmingPhase {
   static const char* phase_name() { return "early graph trimming"; }
   void Run(PipelineData* data, Zone* temp_zone) {

src/compiler/verifier.cc

@@ -194,10 +194,7 @@ void Verifier::Visitor::Check(Node* node) {
       break;
     case IrOpcode::kDead:
       // Dead is never connected to the graph.
-      // TODO(mstarzinger): Make the GraphReducer immediately perform control
-      // reduction in case control is killed. This will prevent {Dead} from
-      // being reachable after a phase finished. Then re-enable below assert.
-      // UNREACHABLE();
+      UNREACHABLE();
       break;
     case IrOpcode::kBranch: {
       // Branch uses are IfTrue and IfFalse.

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

@@ -292,7 +292,7 @@ TEST_F(AdvancedReducerTest, ReplaceWithValue_ValueUse) {
   Node* node = graph()->NewNode(&kMockOperator);
   Node* use_value = graph()->NewNode(common.Return(), node);
   Node* replacement = graph()->NewNode(&kMockOperator);
-  GraphReducer graph_reducer(zone(), graph(), nullptr, nullptr);
+  GraphReducer graph_reducer(zone(), graph(), nullptr);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(replacement, use_value->InputAt(0));
@@ -308,7 +308,7 @@ TEST_F(AdvancedReducerTest, ReplaceWithValue_EffectUse) {
   Node* node = graph()->NewNode(&kMockOpEffect, start);
   Node* use_effect = graph()->NewNode(common.EffectPhi(1), node);
   Node* replacement = graph()->NewNode(&kMockOperator);
-  GraphReducer graph_reducer(zone(), graph(), nullptr, nullptr);
+  GraphReducer graph_reducer(zone(), graph(), nullptr);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(start, use_effect->InputAt(0));
@@ -326,7 +326,7 @@ TEST_F(AdvancedReducerTest, ReplaceWithValue_ControlUse1) {
   Node* success = graph()->NewNode(common.IfSuccess(), node);
   Node* use_control = graph()->NewNode(common.Merge(1), success);
   Node* replacement = graph()->NewNode(&kMockOperator);
-  GraphReducer graph_reducer(zone(), graph(), nullptr, nullptr);
+  GraphReducer graph_reducer(zone(), graph(), nullptr);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(start, use_control->InputAt(0));
@@ -346,19 +346,18 @@ TEST_F(AdvancedReducerTest, ReplaceWithValue_ControlUse2) {
   Node* success = graph()->NewNode(common.IfSuccess(), node);
   Node* exception = graph()->NewNode(common.IfException(kNoHint), effect, node);
   Node* use_control = graph()->NewNode(common.Merge(1), success);
-  Node* use_exception_control = graph()->NewNode(common.Merge(1), exception);
   Node* replacement = graph()->NewNode(&kMockOperator);
-  GraphReducer graph_reducer(zone(), graph(), nullptr, dead);
+  GraphReducer graph_reducer(zone(), graph(), dead);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(start, use_control->InputAt(0));
-  EXPECT_EQ(dead, use_exception_control->InputAt(0));
+  EXPECT_EQ(dead, exception->InputAt(1));
   EXPECT_EQ(0, node->UseCount());
   EXPECT_EQ(2, start->UseCount());
   EXPECT_EQ(1, dead->UseCount());
   EXPECT_EQ(0, replacement->UseCount());
   EXPECT_THAT(start->uses(), UnorderedElementsAre(use_control, node));
-  EXPECT_THAT(dead->uses(), ElementsAre(use_exception_control));
+  EXPECT_THAT(dead->uses(), ElementsAre(exception));
 }
 
 
@@ -371,19 +370,18 @@ TEST_F(AdvancedReducerTest, ReplaceWithValue_ControlUse3) {
   Node* success = graph()->NewNode(common.IfSuccess(), node);
   Node* exception = graph()->NewNode(common.IfException(kNoHint), effect, node);
   Node* use_control = graph()->NewNode(common.Merge(1), success);
-  Node* use_exception_value = graph()->NewNode(common.Return(), exception);
   Node* replacement = graph()->NewNode(&kMockOperator);
-  GraphReducer graph_reducer(zone(), graph(), dead, nullptr);
+  GraphReducer graph_reducer(zone(), graph(), dead);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(start, use_control->InputAt(0));
-  EXPECT_EQ(dead, use_exception_value->InputAt(0));
+  EXPECT_EQ(dead, exception->InputAt(1));
   EXPECT_EQ(0, node->UseCount());
   EXPECT_EQ(2, start->UseCount());
   EXPECT_EQ(1, dead->UseCount());
   EXPECT_EQ(0, replacement->UseCount());
   EXPECT_THAT(start->uses(), UnorderedElementsAre(use_control, node));
-  EXPECT_THAT(dead->uses(), ElementsAre(use_exception_value));
+  EXPECT_THAT(dead->uses(), ElementsAre(exception));
 }