[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

@@ -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);
     }
   }
 }

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<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);

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> 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();
 }
 

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> 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

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

src/compiler/js-inlining.h

@@ -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

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<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);
 

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<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();