From 329a3f220c9b094378b376db8735ef06a66f2024 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Tue, 19 May 2015 02:57:54 -0700 Subject: [PATCH] [turbofan] Turn JSTypedLowering into an AdvancedReducer. This in turn allows usage of AdvancedReducer::ReplaceWithValue which has access to the underlying graph reducer. It will allow us to deal with exception continuations correctly. R=titzer@chromium.org Review URL: https://codereview.chromium.org/1134303003 Cr-Commit-Position: refs/heads/master@{#28468} --- src/compiler/graph-reducer.h | 15 +++++ src/compiler/js-typed-lowering.cc | 67 ++++++++-------------- src/compiler/js-typed-lowering.h | 5 +- src/compiler/pipeline.cc | 4 +- test/cctest/compiler/test-js-typed-lowering.cc | 4 +- .../compiler/js-typed-lowering-unittest.cc | 4 +- 6 files changed, 50 insertions(+), 49 deletions(-) diff --git a/src/compiler/graph-reducer.h b/src/compiler/graph-reducer.h index aee2bca..bd732d5 100644 --- a/src/compiler/graph-reducer.h +++ b/src/compiler/graph-reducer.h @@ -103,6 +103,21 @@ class AdvancedReducer : public Reducer { editor_->ReplaceWithValue(node, value, effect, control); } + // Relax the effects of {node} by immediately replacing effect and control + // uses of {node} with the effect and control input to {node}. + // TODO(turbofan): replace the effect input to {node} with {graph->start()}. + void RelaxEffectsAndControls(Node* node) { + DCHECK_NOT_NULL(editor_); + editor_->ReplaceWithValue(node, node, nullptr, nullptr); + } + + // Relax the control uses of {node} by immediately replacing them with the + // control input to {node}. + void RelaxControls(Node* node) { + DCHECK_NOT_NULL(editor_); + editor_->ReplaceWithValue(node, node, node, nullptr); + } + private: Editor* const editor_; }; diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index 7d8d200..23c269d 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -21,25 +21,8 @@ namespace compiler { // - relax effects from generic but not-side-effecting operations -// Relax the effects of {node} by immediately replacing effect and control uses -// of {node} with the effect and control input to {node}. -// TODO(turbofan): replace the effect input to {node} with {graph->start()}. -// TODO(titzer): move into a GraphEditor? -static void RelaxEffectsAndControls(Node* node) { - NodeProperties::ReplaceWithValue(node, node, NULL); -} - - -// Relax the control uses of {node} by immediately replacing them with the -// control input to {node}. -// TODO(titzer): move into a GraphEditor? -static void RelaxControls(Node* node) { - NodeProperties::ReplaceWithValue(node, node, node); -} - - -JSTypedLowering::JSTypedLowering(JSGraph* jsgraph, Zone* zone) - : jsgraph_(jsgraph), simplified_(graph()->zone()) { +JSTypedLowering::JSTypedLowering(Editor* editor, JSGraph* jsgraph, Zone* zone) + : AdvancedReducer(editor), jsgraph_(jsgraph), simplified_(graph()->zone()) { zero_range_ = Type::Range(0.0, 0.0, graph()->zone()); one_range_ = Type::Range(1.0, 1.0, graph()->zone()); zero_thirtyone_range_ = Type::Range(0.0, 31.0, graph()->zone()); @@ -51,12 +34,6 @@ JSTypedLowering::JSTypedLowering(JSGraph* jsgraph, Zone* zone) } -Reduction JSTypedLowering::ReplaceEagerly(Node* old, Node* node) { - NodeProperties::ReplaceWithValue(old, node, node); - return Changed(node); -} - - // A helper class to construct inline allocations on the simplified operator // level. This keeps track of the effect chain for initial stores on a newly // allocated object and also provides helpers for commonly allocated objects. @@ -189,7 +166,7 @@ class JSBinopReduction final { // Remove the effects from the node, and update its effect/control usages. if (node_->op()->EffectInputCount() > 0) { - RelaxEffectsAndControls(node_); + lowering_->RelaxEffectsAndControls(node_); } // Remove the inputs corresponding to context, effect, and control. NodeProperties::RemoveNonValueInputs(node_); @@ -550,14 +527,18 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node, bool invert) { if (r.left() == r.right()) { // x === x is always true if x != NaN if (!r.left_type()->Maybe(Type::NaN())) { - return ReplaceEagerly(node, jsgraph()->BooleanConstant(!invert)); + Node* replacement = jsgraph()->BooleanConstant(!invert); + Replace(node, replacement); + return Replace(replacement); } } if (r.OneInputCannotBe(Type::NumberOrString())) { // For values with canonical representation (i.e. not string nor number) an // empty type intersection means the values cannot be strictly equal. if (!r.left_type()->Maybe(r.right_type())) { - return ReplaceEagerly(node, jsgraph()->BooleanConstant(invert)); + Node* replacement = jsgraph()->BooleanConstant(invert); + Replace(node, replacement); + return Replace(replacement); } } if (r.OneInputIs(Type::Undefined())) { @@ -615,7 +596,7 @@ Reduction JSTypedLowering::ReduceJSUnaryNot(Node* node) { node->set_op(simplified()->NumberEqual()); node->ReplaceInput(0, length); node->ReplaceInput(1, jsgraph()->ZeroConstant()); - NodeProperties::ReplaceWithValue(node, node, length); + ReplaceWithValue(node, node, length); DCHECK_EQ(2, node->InputCount()); return Changed(node); } @@ -688,7 +669,7 @@ Reduction JSTypedLowering::ReduceJSToNumber(Node* node) { Node* const input = node->InputAt(0); Reduction reduction = ReduceJSToNumberInput(input); if (reduction.Changed()) { - NodeProperties::ReplaceWithValue(node, reduction.replacement()); + ReplaceWithValue(node, reduction.replacement()); return reduction; } Type* const input_type = NodeProperties::GetBounds(input).upper; @@ -741,7 +722,7 @@ Reduction JSTypedLowering::ReduceJSToString(Node* node) { Node* const input = node->InputAt(0); Reduction reduction = ReduceJSToStringInput(input); if (reduction.Changed()) { - NodeProperties::ReplaceWithValue(node, reduction.replacement()); + ReplaceWithValue(node, reduction.replacement()); return reduction; } return NoChange(); @@ -757,7 +738,7 @@ Reduction JSTypedLowering::ReduceJSLoadNamed(Node* node) { Handle constant_value = factory()->GlobalConstantFor(name); if (!constant_value.is_null()) { Node* constant = jsgraph()->Constant(constant_value); - NodeProperties::ReplaceWithValue(node, constant); + ReplaceWithValue(node, constant); return Replace(constant); } } @@ -795,13 +776,15 @@ Reduction JSTypedLowering::ReduceJSLoadProperty(Node* node) { simplified()->LoadElement( AccessBuilder::ForTypedArrayElement(array->type(), true)), buffer, key, effect, control); - return ReplaceEagerly(node, load); + ReplaceWithValue(node, load, load); + return Replace(load); } // Compute byte offset. Node* offset = Word32Shl(key, static_cast(k)); Node* load = graph()->NewNode(simplified()->LoadBuffer(access), buffer, offset, length, effect, control); - return ReplaceEagerly(node, load); + ReplaceWithValue(node, load, load); + return Replace(load); } } } @@ -1039,7 +1022,7 @@ Reduction JSTypedLowering::ReduceJSCreateWithContext(Node* node) { a.Store(AccessBuilder::ForContextSlot(Context::GLOBAL_OBJECT_INDEX), load); // TODO(mstarzinger): We could mutate {node} into the allocation instead. NodeProperties::SetBounds(a.allocation(), NodeProperties::GetBounds(node)); - NodeProperties::ReplaceWithValue(node, node, a.effect()); + ReplaceWithValue(node, node, a.effect()); node->ReplaceInput(0, a.allocation()); node->ReplaceInput(1, a.effect()); node->set_op(common()->Finish(1)); @@ -1078,7 +1061,7 @@ Reduction JSTypedLowering::ReduceJSCreateBlockContext(Node* node) { } // TODO(mstarzinger): We could mutate {node} into the allocation instead. NodeProperties::SetBounds(a.allocation(), NodeProperties::GetBounds(node)); - NodeProperties::ReplaceWithValue(node, node, a.effect()); + ReplaceWithValue(node, node, a.effect()); node->ReplaceInput(0, a.allocation()); node->ReplaceInput(1, a.effect()); node->set_op(common()->Finish(1)); @@ -1097,27 +1080,27 @@ Reduction JSTypedLowering::Reduce(Node* node) { Type* upper = NodeProperties::GetBounds(node).upper; if (upper->IsConstant()) { Node* replacement = jsgraph()->Constant(upper->AsConstant()->Value()); - NodeProperties::ReplaceWithValue(node, replacement); + ReplaceWithValue(node, replacement); return Changed(replacement); } else if (upper->Is(Type::MinusZero())) { Node* replacement = jsgraph()->Constant(factory()->minus_zero_value()); - NodeProperties::ReplaceWithValue(node, replacement); + ReplaceWithValue(node, replacement); return Changed(replacement); } else if (upper->Is(Type::NaN())) { Node* replacement = jsgraph()->NaNConstant(); - NodeProperties::ReplaceWithValue(node, replacement); + ReplaceWithValue(node, replacement); return Changed(replacement); } else if (upper->Is(Type::Null())) { Node* replacement = jsgraph()->NullConstant(); - NodeProperties::ReplaceWithValue(node, replacement); + ReplaceWithValue(node, replacement); return Changed(replacement); } else if (upper->Is(Type::PlainNumber()) && upper->Min() == upper->Max()) { Node* replacement = jsgraph()->Constant(upper->Min()); - NodeProperties::ReplaceWithValue(node, replacement); + ReplaceWithValue(node, replacement); return Changed(replacement); } else if (upper->Is(Type::Undefined())) { Node* replacement = jsgraph()->UndefinedConstant(); - NodeProperties::ReplaceWithValue(node, replacement); + ReplaceWithValue(node, replacement); return Changed(replacement); } } diff --git a/src/compiler/js-typed-lowering.h b/src/compiler/js-typed-lowering.h index b058a08..6533773 100644 --- a/src/compiler/js-typed-lowering.h +++ b/src/compiler/js-typed-lowering.h @@ -26,9 +26,9 @@ class MachineOperatorBuilder; // Lowers JS-level operators to simplified operators based on types. -class JSTypedLowering final : public Reducer { +class JSTypedLowering final : public AdvancedReducer { public: - JSTypedLowering(JSGraph* jsgraph, Zone* zone); + JSTypedLowering(Editor* editor, JSGraph* jsgraph, Zone* zone); ~JSTypedLowering() final {} Reduction Reduce(Node* node) final; @@ -36,7 +36,6 @@ class JSTypedLowering final : public Reducer { private: friend class JSBinopReduction; - Reduction ReplaceEagerly(Node* old, Node* node); Reduction ReduceJSAdd(Node* node); Reduction ReduceJSBitwiseOr(Node* node); Reduction ReduceJSMultiply(Node* node); diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index 7b364a7..f7c5d43 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -557,13 +557,13 @@ struct TypedLoweringPhase { static const char* phase_name() { return "typed lowering"; } void Run(PipelineData* data, Zone* temp_zone) { + GraphReducer graph_reducer(data->graph(), temp_zone); LoadElimination load_elimination; JSBuiltinReducer builtin_reducer(data->jsgraph()); - JSTypedLowering typed_lowering(data->jsgraph(), temp_zone); + JSTypedLowering typed_lowering(&graph_reducer, data->jsgraph(), temp_zone); JSIntrinsicLowering intrinsic_lowering(data->jsgraph()); SimplifiedOperatorReducer simple_reducer(data->jsgraph()); CommonOperatorReducer common_reducer(data->jsgraph()); - GraphReducer graph_reducer(data->graph(), temp_zone); AddReducer(data, &graph_reducer, &builtin_reducer); AddReducer(data, &graph_reducer, &typed_lowering); AddReducer(data, &graph_reducer, &intrinsic_lowering); diff --git a/test/cctest/compiler/test-js-typed-lowering.cc b/test/cctest/compiler/test-js-typed-lowering.cc index 99cc77e..e9cc515 100644 --- a/test/cctest/compiler/test-js-typed-lowering.cc +++ b/test/cctest/compiler/test-js-typed-lowering.cc @@ -89,7 +89,9 @@ class JSTypedLoweringTester : public HandleAndZoneScope { Node* reduce(Node* node) { JSGraph jsgraph(main_isolate(), &graph, &common, &javascript, &machine); - JSTypedLowering reducer(&jsgraph, main_zone()); + // TODO(titzer): mock the GraphReducer here for better unit testing. + GraphReducer graph_reducer(&graph, main_zone()); + JSTypedLowering reducer(&graph_reducer, &jsgraph, main_zone()); Reduction reduction = reducer.Reduce(node); if (reduction.Changed()) return reduction.replacement(); return node; diff --git a/test/unittests/compiler/js-typed-lowering-unittest.cc b/test/unittests/compiler/js-typed-lowering-unittest.cc index 55f23a6..29a4505 100644 --- a/test/unittests/compiler/js-typed-lowering-unittest.cc +++ b/test/unittests/compiler/js-typed-lowering-unittest.cc @@ -80,7 +80,9 @@ class JSTypedLoweringTest : public TypedGraphTest { Reduction Reduce(Node* node) { MachineOperatorBuilder machine(zone()); JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine); - JSTypedLowering reducer(&jsgraph, zone()); + // TODO(titzer): mock the GraphReducer here for better unit testing. + GraphReducer graph_reducer(graph(), zone()); + JSTypedLowering reducer(&graph_reducer, &jsgraph, zone()); return reducer.Reduce(node); } -- 2.7.4