[turbofan] Context specialization is the job of the JSContextSpecialization.
authorbmeurer <bmeurer@chromium.org>
Mon, 6 Jul 2015 12:56:05 +0000 (05:56 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 6 Jul 2015 12:56:28 +0000 (12:56 +0000)
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
src/compiler/ast-graph-builder.h
src/compiler/js-context-specialization.cc
src/compiler/js-context-specialization.h
src/compiler/js-inlining.cc
src/compiler/js-inlining.h
src/compiler/pipeline.cc
test/cctest/compiler/test-js-context-specialization.cc

index 27b1502..31cd3fd 100644 (file)
@@ -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<int>(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<int>(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);
     }
   }
 }
index 298b72d..17190bc 100644 (file)
@@ -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<Node**>(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);
index bb44f59..e4d4d80 100644 (file)
@@ -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> 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();
 }
 
index 1f0c2b8..2ede6b5 100644 (file)
@@ -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> 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> context() const { return context_; }
 
   JSGraph* const jsgraph_;
+  MaybeHandle<Context> context_;
 
-  DISALLOW_COPY_AND_ASSIGN(JSContextSpecializer);
+  DISALLOW_COPY_AND_ASSIGN(JSContextSpecialization);
 };
 
 }  // namespace compiler
index 81bfd98..88d9171 100644 (file)
@@ -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
index 515681f..b075024 100644 (file)
@@ -38,7 +38,8 @@ class JSInliner final : public AdvancedReducer {
                                          Handle<SharedFunctionInfo> 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
index 66f3bf0..ef9adc9 100644 (file)
@@ -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<Code> Pipeline::GenerateCode() {
     Run<LoopAssignmentAnalysisPhase>();
   }
 
-  Run<GraphBuilderPhase>(info()->is_context_specializing());
+  Run<GraphBuilderPhase>();
   if (data.compilation_failed()) return Handle<Code>::null();
   RunPrintAndVerify("Initial untyped", true);
 
index e97da61..328b0ae 100644 (file)
@@ -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<Context>()) {}
 
-  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<Context>());
     graph_reducer.AddReducer(&spec);
     graph_reducer.ReduceGraph();