From 069a47f6e5ed00d0f63fc845b62ad325f13b2c37 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Mon, 6 Jul 2015 05:56:05 -0700 Subject: [PATCH] [turbofan] Context specialization is the job of the JSContextSpecialization. Remove the context specialization hack from the AstGraphBuilder, and properly specialize to the function context in the context specialization. And replace the correct context in the JSInliner. R=mstarzinger@chromium.org BUG=v8:4273 LOG=n Review URL: https://codereview.chromium.org/1218873005 Cr-Commit-Position: refs/heads/master@{#29493} --- src/compiler/ast-graph-builder.cc | 62 +++++++++------------- src/compiler/ast-graph-builder.h | 10 ++-- src/compiler/js-context-specialization.cc | 42 +++++++++++---- src/compiler/js-context-specialization.h | 17 +++--- src/compiler/js-inlining.cc | 19 ++++--- src/compiler/js-inlining.h | 3 +- src/compiler/pipeline.cc | 15 +++--- .../compiler/test-js-context-specialization.cc | 25 ++++----- 8 files changed, 104 insertions(+), 89 deletions(-) diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 27b1502..31cd3fd 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -483,22 +483,19 @@ Node* AstGraphBuilder::GetFunctionClosure() { } -void AstGraphBuilder::CreateFunctionContext(bool constant_context) { - function_context_.set(constant_context - ? jsgraph()->HeapConstant(info()->context()) - : NewOuterContextParam()); -} - - -Node* AstGraphBuilder::NewOuterContextParam() { - // Parameter (arity + 1) is special for the outer context of the function - const Operator* op = - common()->Parameter(info()->num_parameters_including_this(), "%context"); - return NewNode(op, graph()->start()); +Node* AstGraphBuilder::GetFunctionContext() { + if (!function_context_.is_set()) { + // Parameter (arity + 1) is special for the outer context of the function + const Operator* op = common()->Parameter( + info()->num_parameters_including_this(), "%context"); + Node* node = NewNode(op, graph()->start()); + function_context_.set(node); + } + return function_context_.get(); } -bool AstGraphBuilder::CreateGraph(bool constant_context, bool stack_check) { +bool AstGraphBuilder::CreateGraph(bool stack_check) { Scope* scope = info()->scope(); DCHECK(graph() != NULL); @@ -518,8 +515,7 @@ bool AstGraphBuilder::CreateGraph(bool constant_context, bool stack_check) { } // Initialize the incoming context. - CreateFunctionContext(constant_context); - ContextScope incoming(this, scope, function_context_.get()); + ContextScope incoming(this, scope, GetFunctionContext()); // Initialize control scope. ControlScope control(this); @@ -535,7 +531,7 @@ bool AstGraphBuilder::CreateGraph(bool constant_context, bool stack_check) { // Build function context only if there are context allocated variables. if (info()->num_heap_slots() > 0) { // Push a new inner context scope for the function. - Node* inner_context = BuildLocalFunctionContext(function_context_.get()); + Node* inner_context = BuildLocalFunctionContext(GetFunctionContext()); ContextScope top_context(this, scope, inner_context); CreateGraphBody(stack_check); } else { @@ -3631,7 +3627,7 @@ Node* AstGraphBuilder::BuildLoadBuiltinsObject() { Node* AstGraphBuilder::BuildLoadGlobalObject() { const Operator* load_op = javascript()->LoadContext(0, Context::GLOBAL_OBJECT_INDEX, true); - return NewNode(load_op, function_context_.get()); + return NewNode(load_op, GetFunctionContext()); } @@ -4056,11 +4052,9 @@ void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned, if (builder_->info()->is_osr()) { // Introduce phis for all context values in the case of an OSR graph. - for (int i = 0; i < static_cast(contexts()->size()); ++i) { - Node* val = contexts()->at(i); - if (!IrOpcode::IsConstantOpcode(val->opcode())) { - contexts()->at(i) = builder_->NewPhi(1, val, control); - } + for (size_t i = 0; i < contexts()->size(); ++i) { + Node* context = contexts()->at(i); + contexts()->at(i) = builder_->NewPhi(1, context, control); } } @@ -4074,12 +4068,10 @@ void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned, builder_->MergeEffect(effect, osr_loop_entry, control); for (int i = 0; i < size; ++i) { - Node* val = values()->at(i); - if (!IrOpcode::IsConstantOpcode(val->opcode())) { - Node* osr_value = - graph->NewNode(builder_->common()->OsrValue(i), osr_loop_entry); - values()->at(i) = builder_->MergeValue(val, osr_value, control); - } + Node* value = values()->at(i); + Node* osr_value = + graph->NewNode(builder_->common()->OsrValue(i), osr_loop_entry); + values()->at(i) = builder_->MergeValue(value, osr_value, control); } // Rename all the contexts in the environment. @@ -4092,15 +4084,11 @@ void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned, builder_->common()->OsrValue(Linkage::kOsrContextSpillSlotIndex); int last = static_cast(contexts()->size() - 1); for (int i = last; i >= 0; i--) { - Node* val = contexts()->at(i); - if (!IrOpcode::IsConstantOpcode(val->opcode())) { - osr_context = (i == last) ? graph->NewNode(op_inner, osr_loop_entry) - : graph->NewNode(op, osr_context, osr_context, - osr_loop_entry); - contexts()->at(i) = builder_->MergeValue(val, osr_context, control); - } else { - osr_context = val; - } + Node* context = contexts()->at(i); + osr_context = (i == last) ? graph->NewNode(op_inner, osr_loop_entry) + : graph->NewNode(op, osr_context, osr_context, + osr_loop_entry); + contexts()->at(i) = builder_->MergeValue(context, osr_context, control); } } } diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h index 298b72d..17190bc 100644 --- a/src/compiler/ast-graph-builder.h +++ b/src/compiler/ast-graph-builder.h @@ -35,7 +35,7 @@ class AstGraphBuilder : public AstVisitor { JSTypeFeedbackTable* js_type_feedback = NULL); // Creates a graph by visiting the entire AST. - bool CreateGraph(bool constant_context, bool stack_check = true); + bool CreateGraph(bool stack_check = true); // Helpers to create new control nodes. Node* NewIfTrue() { return NewNode(common()->IfTrue()); } @@ -150,13 +150,13 @@ class AstGraphBuilder : public AstVisitor { // Create the main graph body by visiting the AST. void CreateGraphBody(bool stack_check); - // Create the node that represents the outer context of the function. - void CreateFunctionContext(bool constant_context); - // Get or create the node that represents the outer function closure. Node* GetFunctionClosureForContext(); Node* GetFunctionClosure(); + // Get or create the node that represents the outer function context. + Node* GetFunctionContext(); + // Node creation helpers. Node* NewNode(const Operator* op, bool incomplete = false) { return MakeNode(op, 0, static_cast(NULL), incomplete); @@ -202,8 +202,6 @@ class AstGraphBuilder : public AstVisitor { Node* NewPhi(int count, Node* input, Node* control); Node* NewEffectPhi(int count, Node* input, Node* control); - Node* NewOuterContextParam(); - // Helpers for merging control, effect or value dependencies. Node* MergeControl(Node* control, Node* other); Node* MergeEffect(Node* value, Node* other, Node* control); diff --git a/src/compiler/js-context-specialization.cc b/src/compiler/js-context-specialization.cc index bb44f59..e4d4d80 100644 --- a/src/compiler/js-context-specialization.cc +++ b/src/compiler/js-context-specialization.cc @@ -15,18 +15,40 @@ namespace v8 { namespace internal { namespace compiler { -Reduction JSContextSpecializer::Reduce(Node* node) { - if (node->opcode() == IrOpcode::kJSLoadContext) { - return ReduceJSLoadContext(node); +Reduction JSContextSpecialization::Reduce(Node* node) { + switch (node->opcode()) { + case IrOpcode::kParameter: + return ReduceParameter(node); + case IrOpcode::kJSLoadContext: + return ReduceJSLoadContext(node); + case IrOpcode::kJSStoreContext: + return ReduceJSStoreContext(node); + default: + break; } - if (node->opcode() == IrOpcode::kJSStoreContext) { - return ReduceJSStoreContext(node); + return NoChange(); +} + + +Reduction JSContextSpecialization::ReduceParameter(Node* node) { + DCHECK_EQ(IrOpcode::kParameter, node->opcode()); + Node* const start = NodeProperties::GetValueInput(node, 0); + DCHECK_EQ(IrOpcode::kStart, start->opcode()); + int const index = ParameterIndexOf(node->op()); + // The context is always the last parameter to a JavaScript function, and + // {Parameter} indices start at -1, so value outputs of {Start} look like + // this: closure, receiver, param0, ..., paramN, context. + if (index == start->op()->ValueOutputCount() - 2) { + Handle context_constant; + if (context().ToHandle(&context_constant)) { + return Replace(jsgraph()->Constant(context_constant)); + } } return NoChange(); } -Reduction JSContextSpecializer::ReduceJSLoadContext(Node* node) { +Reduction JSContextSpecialization::ReduceJSLoadContext(Node* node) { DCHECK_EQ(IrOpcode::kJSLoadContext, node->opcode()); HeapObjectMatcher m(NodeProperties::GetValueInput(node, 0)); @@ -75,7 +97,7 @@ Reduction JSContextSpecializer::ReduceJSLoadContext(Node* node) { } -Reduction JSContextSpecializer::ReduceJSStoreContext(Node* node) { +Reduction JSContextSpecialization::ReduceJSStoreContext(Node* node) { DCHECK_EQ(IrOpcode::kJSStoreContext, node->opcode()); HeapObjectMatcher m(NodeProperties::GetValueInput(node, 0)); @@ -103,10 +125,12 @@ Reduction JSContextSpecializer::ReduceJSStoreContext(Node* node) { } -Isolate* JSContextSpecializer::isolate() const { return jsgraph()->isolate(); } +Isolate* JSContextSpecialization::isolate() const { + return jsgraph()->isolate(); +} -JSOperatorBuilder* JSContextSpecializer::javascript() const { +JSOperatorBuilder* JSContextSpecialization::javascript() const { return jsgraph()->javascript(); } diff --git a/src/compiler/js-context-specialization.h b/src/compiler/js-context-specialization.h index 1f0c2b8..2ede6b5 100644 --- a/src/compiler/js-context-specialization.h +++ b/src/compiler/js-context-specialization.h @@ -18,25 +18,28 @@ class JSOperatorBuilder; // Specializes a given JSGraph to a given context, potentially constant folding // some {LoadContext} nodes or strength reducing some {StoreContext} nodes. -class JSContextSpecializer : public AdvancedReducer { +class JSContextSpecialization final : public AdvancedReducer { public: - JSContextSpecializer(Editor* editor, JSGraph* jsgraph) - : AdvancedReducer(editor), jsgraph_(jsgraph) {} + JSContextSpecialization(Editor* editor, JSGraph* jsgraph, + MaybeHandle context) + : AdvancedReducer(editor), jsgraph_(jsgraph), context_(context) {} - Reduction Reduce(Node* node) override; + Reduction Reduce(Node* node) final; - // Visible for unit testing. + private: + Reduction ReduceParameter(Node* node); Reduction ReduceJSLoadContext(Node* node); Reduction ReduceJSStoreContext(Node* node); - private: Isolate* isolate() const; JSOperatorBuilder* javascript() const; JSGraph* jsgraph() const { return jsgraph_; } + MaybeHandle context() const { return context_; } JSGraph* const jsgraph_; + MaybeHandle context_; - DISALLOW_COPY_AND_ASSIGN(JSContextSpecializer); + DISALLOW_COPY_AND_ASSIGN(JSContextSpecialization); }; } // namespace compiler diff --git a/src/compiler/js-inlining.cc b/src/compiler/js-inlining.cc index 81bfd98..88d9171 100644 --- a/src/compiler/js-inlining.cc +++ b/src/compiler/js-inlining.cc @@ -115,8 +115,8 @@ class CopyVisitor { }; -Reduction JSInliner::InlineCall(Node* call, Node* frame_state, Node* start, - Node* end) { +Reduction JSInliner::InlineCall(Node* call, Node* context, Node* frame_state, + Node* start, Node* end) { // The scheduler is smart enough to place our code; we just ensure {control} // becomes the control input of the start of the inlinee, and {effect} becomes // the effect input of the start of the inlinee. @@ -139,14 +139,12 @@ Reduction JSInliner::InlineCall(Node* call, Node* frame_state, Node* start, if (index < inliner_inputs && index < inlinee_context_index) { // There is an input from the call, and the index is a value // projection but not the context, so rewire the input. - ReplaceWithValue(use, call->InputAt(index)); + Replace(use, call->InputAt(index)); } else if (index == inlinee_context_index) { - // TODO(turbofan): We always context specialize inlinees currently, so - // we should never get here. - UNREACHABLE(); + Replace(use, context); } else if (index < inlinee_context_index) { // Call has fewer arguments than required, fill with undefined. - ReplaceWithValue(use, jsgraph_->UndefinedConstant()); + Replace(use, jsgraph_->UndefinedConstant()); } else { // We got too many arguments, discard for now. // TODO(sigurds): Fix to treat arguments array correctly. @@ -307,13 +305,14 @@ Reduction JSInliner::Reduce(Node* node) { Graph graph(info.zone()); JSGraph jsgraph(info.isolate(), &graph, jsgraph_->common(), jsgraph_->javascript(), jsgraph_->machine()); + AstGraphBuilder graph_builder(local_zone_, &info, &jsgraph); + graph_builder.CreateGraph(false); // The inlinee specializes to the context from the JSFunction object. // TODO(turbofan): We might want to load the context from the JSFunction at // runtime in case we only know the SharedFunctionInfo once we have dynamic // type feedback in the compiler. - AstGraphBuilder graph_builder(local_zone_, &info, &jsgraph); - graph_builder.CreateGraph(true, false); + Node* context = jsgraph_->Constant(handle(function->context())); CopyVisitor visitor(&graph, jsgraph_->graph(), info.zone()); visitor.CopyGraph(); @@ -338,7 +337,7 @@ Reduction JSInliner::Reduce(Node* node) { // Remember that we inlined this function. info_->AddInlinedFunction(info.shared_info()); - return InlineCall(node, frame_state, start, end); + return InlineCall(node, context, frame_state, start, end); } } // namespace compiler diff --git a/src/compiler/js-inlining.h b/src/compiler/js-inlining.h index 515681f..b075024 100644 --- a/src/compiler/js-inlining.h +++ b/src/compiler/js-inlining.h @@ -38,7 +38,8 @@ class JSInliner final : public AdvancedReducer { Handle shared_info, Zone* temp_zone); - Reduction InlineCall(Node* call, Node* frame_state, Node* start, Node* end); + Reduction InlineCall(Node* call, Node* context, Node* frame_state, + Node* start, Node* end); }; } // namespace compiler diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index 66f3bf0..ef9adc9 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -366,9 +366,9 @@ class AstGraphBuilderWithPositions final : public AstGraphBuilder { source_positions_(source_positions), start_position_(info->shared_info()->start_position()) {} - bool CreateGraph(bool constant_context, bool stack_check) { + bool CreateGraph(bool stack_check) { SourcePositionTable::Scope pos_scope(source_positions_, start_position_); - return AstGraphBuilder::CreateGraph(constant_context, stack_check); + return AstGraphBuilder::CreateGraph(stack_check); } #define DEF_VISIT(type) \ @@ -475,12 +475,12 @@ struct LoopAssignmentAnalysisPhase { struct GraphBuilderPhase { static const char* phase_name() { return "graph builder"; } - void Run(PipelineData* data, Zone* temp_zone, bool constant_context) { + void Run(PipelineData* data, Zone* temp_zone) { AstGraphBuilderWithPositions graph_builder( temp_zone, data->info(), data->jsgraph(), data->loop_assignment(), data->js_type_feedback(), data->source_positions()); bool stack_check = !data->info()->IsStub(); - if (!graph_builder.CreateGraph(constant_context, stack_check)) { + if (!graph_builder.CreateGraph(stack_check)) { data->set_compilation_failed(); } } @@ -496,7 +496,8 @@ struct InliningPhase { data->common()); CommonOperatorReducer common_reducer(&graph_reducer, data->graph(), data->common(), data->machine()); - JSContextSpecializer context_specializer(&graph_reducer, data->jsgraph()); + JSContextSpecialization context_specialization( + &graph_reducer, data->jsgraph(), data->info()->context()); JSFrameSpecialization frame_specialization(data->info()->osr_frame(), data->jsgraph()); JSInliner inliner(&graph_reducer, data->info()->is_inlining_enabled() @@ -509,7 +510,7 @@ struct InliningPhase { AddReducer(data, &graph_reducer, &frame_specialization); } if (data->info()->is_context_specializing()) { - AddReducer(data, &graph_reducer, &context_specializer); + AddReducer(data, &graph_reducer, &context_specialization); } AddReducer(data, &graph_reducer, &inliner); graph_reducer.ReduceGraph(); @@ -1037,7 +1038,7 @@ Handle Pipeline::GenerateCode() { Run(); } - Run(info()->is_context_specializing()); + Run(); if (data.compilation_failed()) return Handle::null(); RunPrintAndVerify("Initial untyped", true); diff --git a/test/cctest/compiler/test-js-context-specialization.cc b/test/cctest/compiler/test-js-context-specialization.cc index e97da61..328b0ae 100644 --- a/test/cctest/compiler/test-js-context-specialization.cc +++ b/test/cctest/compiler/test-js-context-specialization.cc @@ -25,9 +25,9 @@ class ContextSpecializationTester : public HandleAndZoneScope { simplified_(main_zone()), jsgraph_(main_isolate(), graph(), common(), &javascript_, &machine_), reducer_(main_zone(), graph()), - spec_(&reducer_, jsgraph()) {} + spec_(&reducer_, jsgraph(), MaybeHandle()) {} - JSContextSpecializer* spec() { return &spec_; } + JSContextSpecialization* spec() { return &spec_; } Factory* factory() { return main_isolate()->factory(); } CommonOperatorBuilder* common() { return &common_; } JSOperatorBuilder* javascript() { return &javascript_; } @@ -43,7 +43,7 @@ class ContextSpecializationTester : public HandleAndZoneScope { SimplifiedOperatorBuilder simplified_; JSGraph jsgraph_; GraphReducer reducer_; - JSContextSpecializer spec_; + JSContextSpecialization spec_; }; @@ -71,7 +71,7 @@ TEST(ReduceJSLoadContext) { // Mutable slot, constant context, depth = 0 => do nothing. Node* load = t.graph()->NewNode(t.javascript()->LoadContext(0, 0, false), const_context, const_context, start); - Reduction r = t.spec()->ReduceJSLoadContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(!r.Changed()); } @@ -79,7 +79,7 @@ TEST(ReduceJSLoadContext) { // Mutable slot, non-constant context, depth = 0 => do nothing. Node* load = t.graph()->NewNode(t.javascript()->LoadContext(0, 0, false), param_context, param_context, start); - Reduction r = t.spec()->ReduceJSLoadContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(!r.Changed()); } @@ -88,7 +88,7 @@ TEST(ReduceJSLoadContext) { Node* load = t.graph()->NewNode( t.javascript()->LoadContext(2, Context::GLOBAL_EVAL_FUN_INDEX, false), deep_const_context, deep_const_context, start); - Reduction r = t.spec()->ReduceJSLoadContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(r.Changed()); Node* new_context_input = NodeProperties::GetValueInput(r.replacement(), 0); CHECK_EQ(IrOpcode::kHeapConstant, new_context_input->opcode()); @@ -104,7 +104,7 @@ TEST(ReduceJSLoadContext) { // Immutable slot, constant context, depth = 0 => specialize. Node* load = t.graph()->NewNode(t.javascript()->LoadContext(0, slot, true), const_context, const_context, start); - Reduction r = t.spec()->ReduceJSLoadContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(r.Changed()); CHECK(r.replacement() != load); @@ -142,7 +142,7 @@ TEST(ReduceJSStoreContext) { // Mutable slot, constant context, depth = 0 => do nothing. Node* load = t.graph()->NewNode(t.javascript()->StoreContext(0, 0), const_context, const_context, start); - Reduction r = t.spec()->ReduceJSStoreContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(!r.Changed()); } @@ -150,7 +150,7 @@ TEST(ReduceJSStoreContext) { // Mutable slot, non-constant context, depth = 0 => do nothing. Node* load = t.graph()->NewNode(t.javascript()->StoreContext(0, 0), param_context, param_context, start); - Reduction r = t.spec()->ReduceJSStoreContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(!r.Changed()); } @@ -158,7 +158,7 @@ TEST(ReduceJSStoreContext) { // Immutable slot, constant context, depth = 0 => do nothing. Node* load = t.graph()->NewNode(t.javascript()->StoreContext(0, slot), const_context, const_context, start); - Reduction r = t.spec()->ReduceJSStoreContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(!r.Changed()); } @@ -167,7 +167,7 @@ TEST(ReduceJSStoreContext) { Node* load = t.graph()->NewNode( t.javascript()->StoreContext(2, Context::GLOBAL_EVAL_FUN_INDEX), deep_const_context, deep_const_context, start); - Reduction r = t.spec()->ReduceJSStoreContext(load); + Reduction r = t.spec()->Reduce(load); CHECK(r.Changed()); Node* new_context_input = NodeProperties::GetValueInput(r.replacement(), 0); CHECK_EQ(IrOpcode::kHeapConstant, new_context_input->opcode()); @@ -235,7 +235,8 @@ TEST(SpecializeToContext) { // Perform the reduction on the entire graph. GraphReducer graph_reducer(t.main_zone(), t.graph()); - JSContextSpecializer spec(&graph_reducer, t.jsgraph()); + JSContextSpecialization spec(&graph_reducer, t.jsgraph(), + MaybeHandle()); graph_reducer.AddReducer(&spec); graph_reducer.ReduceGraph(); -- 2.7.4