Reviewers: Michael Starzinger,

Description:
[turbofan] Context specialization should only specialize loads/stores.

The JSContextSpecialization should only care about loads from the
context and stores to the context, where the context is either a
HeapConstant or the special context Parameter (and a context for the
outer most function is provided). This way we don't eagerly embed
arbitrary context constants for no benefit, but we still specialize the
loads and store which we actually care about.

[email protected]

Please review this at https://codereview.chromium.org/1227963005/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+38, -36 lines):
  M src/compiler/js-context-specialization.h
  M src/compiler/js-context-specialization.cc
  M src/compiler/pipeline.cc


Index: src/compiler/js-context-specialization.cc
diff --git a/src/compiler/js-context-specialization.cc b/src/compiler/js-context-specialization.cc index e4d4d80f52f9daea326dbf113223d3edf6c2a4e4..07746fa98bbed622c4e363a6eedd78703041035b 100644
--- a/src/compiler/js-context-specialization.cc
+++ b/src/compiler/js-context-specialization.cc
@@ -17,8 +17,6 @@ namespace compiler {

 Reduction JSContextSpecialization::Reduce(Node* node) {
   switch (node->opcode()) {
-    case IrOpcode::kParameter:
-      return ReduceParameter(node);
     case IrOpcode::kJSLoadContext:
       return ReduceJSLoadContext(node);
     case IrOpcode::kJSStoreContext:
@@ -30,37 +28,43 @@ Reduction JSContextSpecialization::Reduce(Node* node) {
 }


-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));
+MaybeHandle<Context> JSContextSpecialization::GetSpecializationContext(
+    Node* node) {
+  DCHECK(node->opcode() == IrOpcode::kJSLoadContext ||
+         node->opcode() == IrOpcode::kJSStoreContext);
+  Node* const object = NodeProperties::GetValueInput(node, 0);
+  switch (object->opcode()) {
+    case IrOpcode::kHeapConstant:
+      return Handle<Context>::cast(
+          OpParameter<Unique<HeapObject>>(object).handle());
+    case IrOpcode::kParameter: {
+      Node* const start = NodeProperties::GetValueInput(object, 0);
+      DCHECK_EQ(IrOpcode::kStart, start->opcode());
+      int const index = ParameterIndexOf(object->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) {
+        return context();
+      }
+      break;
     }
+    default:
+      break;
   }
-  return NoChange();
+  return MaybeHandle<Context>();
 }


 Reduction JSContextSpecialization::ReduceJSLoadContext(Node* node) {
   DCHECK_EQ(IrOpcode::kJSLoadContext, node->opcode());

-  HeapObjectMatcher m(NodeProperties::GetValueInput(node, 0));
-  // If the context is not constant, no reduction can occur.
-  if (!m.HasValue()) {
-    return NoChange();
-  }
-
-  const ContextAccess& access = ContextAccessOf(node->op());
+  // Get the specialization context from the node.
+  Handle<Context> context;
+ if (!GetSpecializationContext(node).ToHandle(&context)) return NoChange();

   // Find the right parent context.
-  Handle<Context> context = Handle<Context>::cast(m.Value().handle());
+  const ContextAccess& access = ContextAccessOf(node->op());
   for (size_t i = access.depth(); i > 0; --i) {
     context = handle(context->previous(), isolate());
   }
@@ -100,21 +104,17 @@ Reduction JSContextSpecialization::ReduceJSLoadContext(Node* node) {
 Reduction JSContextSpecialization::ReduceJSStoreContext(Node* node) {
   DCHECK_EQ(IrOpcode::kJSStoreContext, node->opcode());

-  HeapObjectMatcher m(NodeProperties::GetValueInput(node, 0));
-  // If the context is not constant, no reduction can occur.
-  if (!m.HasValue()) {
-    return NoChange();
-  }
-
-  const ContextAccess& access = ContextAccessOf(node->op());
+  // Get the specialization context from the node.
+  Handle<Context> context;
+ if (!GetSpecializationContext(node).ToHandle(&context)) return NoChange();

   // The access does not have to look up a parent, nothing to fold.
+  const ContextAccess& access = ContextAccessOf(node->op());
   if (access.depth() == 0) {
     return NoChange();
   }

   // Find the right parent context.
-  Handle<Context> context = Handle<Context>::cast(m.Value().handle());
   for (size_t i = access.depth(); i > 0; --i) {
     context = handle(context->previous(), isolate());
   }
Index: src/compiler/js-context-specialization.h
diff --git a/src/compiler/js-context-specialization.h b/src/compiler/js-context-specialization.h index 2ede6b5e1767f163396c057fc5fbe6ad1061d7ad..ef784fc442812ca1c9a7cac1622a4c194a389786 100644
--- a/src/compiler/js-context-specialization.h
+++ b/src/compiler/js-context-specialization.h
@@ -27,10 +27,12 @@ class JSContextSpecialization final : public AdvancedReducer {
   Reduction Reduce(Node* node) final;

  private:
-  Reduction ReduceParameter(Node* node);
   Reduction ReduceJSLoadContext(Node* node);
   Reduction ReduceJSStoreContext(Node* node);

+  // Returns the {Context} to specialize {node} to (if any).
+  MaybeHandle<Context> GetSpecializationContext(Node* node);
+
   Isolate* isolate() const;
   JSOperatorBuilder* javascript() const;
   JSGraph* jsgraph() const { return jsgraph_; }
Index: src/compiler/pipeline.cc
diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
index b816f533f8a35f3401967b23416bfc566c269a40..b8dc459e0e30ab8dabf3fea0c7ae4f89d6680f33 100644
--- a/src/compiler/pipeline.cc
+++ b/src/compiler/pipeline.cc
@@ -498,7 +498,9 @@ struct InliningPhase {
     CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
                                          data->common(), data->machine());
     JSContextSpecialization context_specialization(
-        &graph_reducer, data->jsgraph(), data->info()->context());
+ &graph_reducer, data->jsgraph(), data->info()->is_context_specializing()
+                                             ? data->info()->context()
+                                             : MaybeHandle<Context>());
     JSFrameSpecialization frame_specialization(data->info()->osr_frame(),
                                                data->jsgraph());
     JSInliner inliner(&graph_reducer, data->info()->is_inlining_enabled()
@@ -510,9 +512,7 @@ struct InliningPhase {
     if (data->info()->is_frame_specializing()) {
       AddReducer(data, &graph_reducer, &frame_specialization);
     }
-    if (data->info()->is_context_specializing()) {
-      AddReducer(data, &graph_reducer, &context_specialization);
-    }
+    AddReducer(data, &graph_reducer, &context_specialization);
     AddReducer(data, &graph_reducer, &inliner);
     graph_reducer.ReduceGraph();
   }


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to