From 64d6ab455d5749f7d8c93f99943e0ef67a2266bc Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Tue, 23 Jun 2015 13:21:51 +0200 Subject: [PATCH] [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 | 9 +++-- src/compiler/common-operator-reducer.h | 1 + src/compiler/graph-reducer.cc | 18 ++++------ src/compiler/graph-reducer.h | 6 ++-- src/compiler/js-intrinsic-lowering.cc | 33 ++++++------------- src/compiler/pipeline.cc | 26 ++++++--------- src/compiler/verifier.cc | 5 +-- .../compiler/graph-reducer-unittest.cc | 20 +++++------ 8 files changed, 44 insertions(+), 74 deletions(-) diff --git a/src/compiler/common-operator-reducer.cc b/src/compiler/common-operator-reducer.cc index 14bf812243..52b9ca8389 100644 --- a/src/compiler/common-operator-reducer.cc +++ b/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()); } diff --git a/src/compiler/common-operator-reducer.h b/src/compiler/common-operator-reducer.h index 1456c15e95..3c43bda455 100644 --- a/src/compiler/common-operator-reducer.h +++ b/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 dead_; }; } // namespace compiler diff --git a/src/compiler/graph-reducer.cc b/src/compiler/graph-reducer.cc index 33d400b61e..80b40a7d9a 100644 --- a/src/compiler/graph-reducer.cc +++ b/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(); } diff --git a/src/compiler/graph-reducer.h b/src/compiler/graph-reducer.h index b4e9ef8d1c..39c302f892 100644 --- a/src/compiler/graph-reducer.h +++ b/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_; ZoneVector reducers_; ZoneStack revisit_; diff --git a/src/compiler/js-intrinsic-lowering.cc b/src/compiler/js-intrinsic-lowering.cc index 1e3de48799..f4a886f88c 100644 --- a/src/compiler/js-intrinsic-lowering.cc +++ b/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* const frame_state = NodeProperties::GetFrameStateInput(node, 0); + Node* const effect = NodeProperties::GetEffectInput(node); + Node* const control = NodeProperties::GetControlInput(node); - Node* effect = NodeProperties::GetEffectInput(node); - Node* control = NodeProperties::GetControlInput(node); + // TODO(bmeurer): Move MergeControlToEnd() to the AdvancedReducer. + Node* deoptimize = + graph()->NewNode(common()->Deoptimize(), frame_state, effect, control); + NodeProperties::MergeControlToEnd(graph(), common(), deoptimize); - // 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); + node->set_op(common()->Dead()); + node->TrimInputCount(0); + return Changed(node); } diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index d71ade0ce0..81a00dd18a 100644 --- a/src/compiler/pipeline.cc +++ b/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) { diff --git a/src/compiler/verifier.cc b/src/compiler/verifier.cc index e678f4e6a5..3cc1e85c0a 100644 --- a/src/compiler/verifier.cc +++ b/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. diff --git a/test/unittests/compiler/graph-reducer-unittest.cc b/test/unittests/compiler/graph-reducer-unittest.cc index 2d54e21442..3ca6052af9 100644 --- a/test/unittests/compiler/graph-reducer-unittest.cc +++ b/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)); }