Revert of [turbofan] Run DeadCodeElimination together with the advanced...

Revert of [turbofan] Run DeadCodeElimination together with the advanced reducers. (patchset #1 id:1 of https://codereview.chromium.org/1206533002/)

Reason for revert:
Looks like this breaks Tests262.

Original issue's description:
> [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}

TBR=jarin@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

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

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

src/compiler/graph-reducer.cc

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

src/compiler/graph-reducer.h

@@ -116,7 +116,8 @@ 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 = nullptr);
+  GraphReducer(Zone* zone, Graph* graph, Node* dead_value = nullptr,
+               Node* dead_control = nullptr);
   ~GraphReducer();
 
   Graph* graph() const { return graph_; }
@@ -163,7 +164,8 @@ class GraphReducer : public AdvancedReducer::Editor {
   void Revisit(Node* node) final;
 
   Graph* const graph_;
-  Node* const dead_;
+  Node* dead_value_;
+  Node* dead_control_;
   NodeMarker<State> state_;
   ZoneVector<Reducer*> reducers_;
   ZoneStack<Node*> revisit_;

src/compiler/js-intrinsic-lowering.cc

@@ -132,18 +132,31 @@ Reduction JSIntrinsicLowering::ReduceDateField(Node* node) {
 
 Reduction JSIntrinsicLowering::ReduceDeoptimizeNow(Node* node) {
   if (mode() != kDeoptimizationEnabled) return NoChange();
-  Node* const frame_state = NodeProperties::GetFrameStateInput(node, 0);
-  Node* const effect = NodeProperties::GetEffectInput(node);
-  Node* const control = NodeProperties::GetControlInput(node);
+  Node* frame_state = NodeProperties::GetFrameStateInput(node, 0);
+  DCHECK_EQ(frame_state->opcode(), IrOpcode::kFrameState);
 
-  // TODO(bmeurer): Move MergeControlToEnd() to the AdvancedReducer.
-  Node* deoptimize =
-      graph()->NewNode(common()->Deoptimize(), frame_state, effect, control);
-  NodeProperties::MergeControlToEnd(graph(), common(), deoptimize);
+  Node* effect = NodeProperties::GetEffectInput(node);
+  Node* control = NodeProperties::GetControlInput(node);
 
-  node->set_op(common()->Dead());
-  node->TrimInputCount(0);
-  return Changed(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);
+
+  // Connect the deopt to the merge exiting the graph.
+  NodeProperties::MergeControlToEnd(graph(), common(), deopt);
+
+  return Changed(deopt);
 }
 
 

src/compiler/pipeline.cc

@@ -407,7 +407,8 @@ class SourcePositionWrapper final : public Reducer {
 class JSGraphReducer final : public GraphReducer {
  public:
   JSGraphReducer(JSGraph* jsgraph, Zone* zone)
-      : GraphReducer(zone, jsgraph->graph(), jsgraph->Dead()) {}
+      : GraphReducer(zone, jsgraph->graph(), jsgraph->TheHoleConstant(),
+                     jsgraph->Dead()) {}
   ~JSGraphReducer() final {}
 };
 
@@ -564,8 +565,6 @@ 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);
@@ -576,7 +575,6 @@ 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);
@@ -595,13 +593,10 @@ 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);
@@ -626,14 +621,11 @@ 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);
@@ -643,6 +635,20 @@ 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,7 +194,10 @@ void Verifier::Visitor::Check(Node* node) {
       break;
     case IrOpcode::kDead:
       // Dead is never connected to the graph.
-      UNREACHABLE();
+      // 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();
       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);
+  GraphReducer graph_reducer(zone(), graph(), nullptr, 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);
+  GraphReducer graph_reducer(zone(), graph(), nullptr, 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);
+  GraphReducer graph_reducer(zone(), graph(), nullptr, nullptr);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(start, use_control->InputAt(0));
@@ -346,18 +346,19 @@ 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(), dead);
+  GraphReducer graph_reducer(zone(), graph(), nullptr, dead);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(start, use_control->InputAt(0));
-  EXPECT_EQ(dead, exception->InputAt(1));
+  EXPECT_EQ(dead, use_exception_control->InputAt(0));
   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(exception));
+  EXPECT_THAT(dead->uses(), ElementsAre(use_exception_control));
 }
 
 
@@ -370,18 +371,19 @@ 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);
+  GraphReducer graph_reducer(zone(), graph(), dead, nullptr);
   ReplaceWithValueReducer r(&graph_reducer);
   r.ReplaceWithValue(node, replacement);
   EXPECT_EQ(start, use_control->InputAt(0));
-  EXPECT_EQ(dead, exception->InputAt(1));
+  EXPECT_EQ(dead, use_exception_value->InputAt(0));
   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(exception));
+  EXPECT_THAT(dead->uses(), ElementsAre(use_exception_value));
 }