[ic] Collect new.target feedback for Construct bytecodes.

Change the CALL_IC machinery inside of Ignition to collect new.target
feedback for Construct and ConstructWithSpread bytecodes instead of
collecting feedback about the target, and adapt TurboFan's JSCallReducer
to consume feedback for new.target instead of target on JSConstruct
nodes.

This enables TurboFan to inline JSCreate - and thus the actual instance
allocation - into derived leaf constructors even if the leaf constructor
itself is not inlined, and thereby removes this weird performance cliff.
The feedback for target in case of class constructors is provided by
the function context specialization, and in case of `new A`, we can
just use the feedback for new.target, as both target and new.target are
A in that case.

Bug: v8:5517, v8:6399, v8:6679
Change-Id: I0475e2500e787fd672ed037ac0faed78a8fa5dc0
Reviewed-on: https://chromium-review.googlesource.com/604790Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47210}

src/compiler/js-call-reducer.cc

@@ -1371,58 +1371,13 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
-  // Try to specialize JSConstruct {node}s with constant {target}s.
-  HeapObjectMatcher m(target);
-  if (m.HasValue()) {
-    if (m.Value()->IsJSFunction()) {
-      Handle<JSFunction> function = Handle<JSFunction>::cast(m.Value());
-
-      // Raise a TypeError if the {target} is not a constructor.
-      if (!function->IsConstructor()) {
-        NodeProperties::ReplaceValueInputs(node, target);
-        NodeProperties::ChangeOp(
-            node, javascript()->CallRuntime(
-                      Runtime::kThrowConstructedNonConstructable));
-        return Changed(node);
-      }
-
-      // Don't inline cross native context.
-      if (function->native_context() != *native_context()) return NoChange();
-
-      // Check for the ArrayConstructor.
-      if (*function == function->native_context()->array_function()) {
-        // Check if we have an allocation site.
-        Handle<AllocationSite> site;
-        if (p.feedback().IsValid()) {
-          CallICNexus nexus(p.feedback().vector(), p.feedback().slot());
-          Handle<Object> feedback(nexus.GetFeedback(), isolate());
-          if (feedback->IsAllocationSite()) {
-            site = Handle<AllocationSite>::cast(feedback);
-          }
-        }
-
-        // Turn the {node} into a {JSCreateArray} call.
-        for (int i = arity; i > 0; --i) {
-          NodeProperties::ReplaceValueInput(
-              node, NodeProperties::GetValueInput(node, i), i + 1);
-        }
-        NodeProperties::ReplaceValueInput(node, new_target, 1);
-        NodeProperties::ChangeOp(node, javascript()->CreateArray(arity, site));
-        return Changed(node);
-      }
-    }
-
-    // Don't mess with other {node}s that have a constant {target}.
-    // TODO(bmeurer): Also support optimizing bound functions and proxies here.
-    return NoChange();
-  }
-
   // Extract feedback from the {node} using the CallICNexus.
-  if (!p.feedback().IsValid()) return NoChange();
+  if (p.feedback().IsValid()) {
     CallICNexus nexus(p.feedback().vector(), p.feedback().slot());
     if (nexus.IsUninitialized()) {
       if (flags() & kBailoutOnUninitialized) {
-      // Introduce a SOFT deopt if the construct {node} wasn't executed so far.
+        // Introduce a SOFT deopt if the construct {node} wasn't executed so
+        // far.
         return ReduceSoftDeoptimize(
             node, DeoptimizeReason::kInsufficientTypeFeedbackForConstruct);
       }
@@ -1444,7 +1399,8 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
       // Check that the {target} is still the {array_function}.
       Node* check = graph()->NewNode(simplified()->ReferenceEqual(), target,
                                      array_function);
-    effect = graph()->NewNode(simplified()->CheckIf(), check, effect, control);
+      effect =
+          graph()->NewNode(simplified()->CheckIf(), check, effect, control);
 
       // Turn the {node} into a {JSCreateArray} call.
       NodeProperties::ReplaceEffectInput(node, effect);
@@ -1452,29 +1408,29 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
         NodeProperties::ReplaceValueInput(
             node, NodeProperties::GetValueInput(node, i), i + 1);
       }
-    NodeProperties::ReplaceValueInput(node, new_target, 1);
+      NodeProperties::ReplaceValueInput(node, array_function, 1);
       NodeProperties::ChangeOp(node, javascript()->CreateArray(arity, site));
       return Changed(node);
     } else if (feedback->IsWeakCell()) {
       // Check if we want to use CallIC feedback here.
-    if (!ShouldUseCallICFeedback(target)) return NoChange();
+      if (!ShouldUseCallICFeedback(new_target)) return NoChange();
 
       Handle<WeakCell> cell = Handle<WeakCell>::cast(feedback);
-    if (cell->value()->IsJSFunction()) {
-      Node* target_function =
+      if (cell->value()->IsConstructor()) {
+        Node* new_target_feedback =
             jsgraph()->Constant(handle(cell->value(), isolate()));
 
-      // Check that the {target} is still the {target_function}.
-      Node* check = graph()->NewNode(simplified()->ReferenceEqual(), target,
-                                     target_function);
+        // Check that the {new_target} is still the {new_target_feedback}.
+        Node* check = graph()->NewNode(simplified()->ReferenceEqual(),
+                                       new_target, new_target_feedback);
         effect =
             graph()->NewNode(simplified()->CheckIf(), check, effect, control);
 
-      // Specialize the JSConstruct node to the {target_function}.
-      NodeProperties::ReplaceValueInput(node, target_function, 0);
+        // Specialize the JSConstruct node to the {new_target_feedback}.
+        NodeProperties::ReplaceValueInput(node, new_target_feedback, arity + 1);
         NodeProperties::ReplaceEffectInput(node, effect);
         if (target == new_target) {
-        NodeProperties::ReplaceValueInput(node, target_function, arity + 1);
+          NodeProperties::ReplaceValueInput(node, new_target_feedback, 0);
         }
 
         // Try to further reduce the JSConstruct {node}.
@@ -1482,6 +1438,43 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
         return reduction.Changed() ? reduction : Changed(node);
       }
     }
+  }
+
+  // Try to specialize JSConstruct {node}s with constant {target}s.
+  HeapObjectMatcher m(target);
+  if (m.HasValue()) {
+    if (m.Value()->IsJSFunction()) {
+      Handle<JSFunction> function = Handle<JSFunction>::cast(m.Value());
+
+      // Raise a TypeError if the {target} is not a constructor.
+      if (!function->IsConstructor()) {
+        NodeProperties::ReplaceValueInputs(node, target);
+        NodeProperties::ChangeOp(
+            node, javascript()->CallRuntime(
+                      Runtime::kThrowConstructedNonConstructable));
+        return Changed(node);
+      }
+
+      // Don't inline cross native context.
+      if (function->native_context() != *native_context()) return NoChange();
+
+      // Check for the ArrayConstructor.
+      if (*function == function->native_context()->array_function()) {
+        // TODO(bmeurer): Deal with Array subclasses here.
+        Handle<AllocationSite> site;
+        // Turn the {node} into a {JSCreateArray} call.
+        for (int i = arity; i > 0; --i) {
+          NodeProperties::ReplaceValueInput(
+              node, NodeProperties::GetValueInput(node, i), i + 1);
+        }
+        NodeProperties::ReplaceValueInput(node, new_target, 1);
+        NodeProperties::ChangeOp(node, javascript()->CreateArray(arity, site));
+        return Changed(node);
+      }
+    }
+
+    // TODO(bmeurer): Also support optimizing bound functions and proxies here.
+  }
 
   return NoChange();
 }

src/interpreter/interpreter-assembler.h

@@ -119,9 +119,10 @@ class V8_EXPORT_PRIVATE InterpreterAssembler : public CodeStubAssembler {
   compiler::Node* IncrementCallCount(compiler::Node* feedback_vector,
                                      compiler::Node* slot_id);
 
-  // Collect CALL_IC feedback for |target| function in the
+  // Collect CALL_IC feedback for |target_or_new_target| function in the
   // |feedback_vector| at |slot_id|.
-  void CollectCallFeedback(compiler::Node* target, compiler::Node* context,
+  void CollectCallOrConstructFeedback(compiler::Node* target_or_new_target,
+                                      compiler::Node* context,
                                       compiler::Node* slot_id,
                                       compiler::Node* feedback_vector);
 
@@ -142,22 +143,22 @@ class V8_EXPORT_PRIVATE InterpreterAssembler : public CodeStubAssembler {
                                    compiler::Node* slot_id,
                                    compiler::Node* feedback_vector);
 
-  // Call constructor |constructor| with |arg_count| arguments (not
+  // Call constructor |target| with |arg_count| arguments (not
   // including receiver) and the first argument located at
   // |first_arg|. The |new_target| is the same as the
-  // |constructor| for the new keyword, but differs for the super
+  // |target| for the new keyword, but differs for the super
   // keyword.
-  compiler::Node* Construct(compiler::Node* constructor,
-                            compiler::Node* context, compiler::Node* new_target,
+  compiler::Node* Construct(compiler::Node* target, compiler::Node* context,
+                            compiler::Node* new_target,
                             compiler::Node* first_arg,
                             compiler::Node* arg_count, compiler::Node* slot_id,
                             compiler::Node* feedback_vector);
 
-  // Call constructor |constructor| with |arg_count| arguments (not including
+  // Call constructor |target| with |arg_count| arguments (not including
   // receiver) and the first argument located at |first_arg|. The last argument
-  // is always a spread. The |new_target| is the same as the |constructor| for
+  // is always a spread. The |new_target| is the same as the |target| for
   // the new keyword, but differs for the super keyword.
-  compiler::Node* ConstructWithSpread(compiler::Node* constructor,
+  compiler::Node* ConstructWithSpread(compiler::Node* target,
                                       compiler::Node* context,
                                       compiler::Node* new_target,
                                       compiler::Node* first_arg,

src/interpreter/interpreter-generator.cc

@@ -1720,7 +1720,7 @@ class InterpreterJSCallAssembler : public InterpreterAssembler {
     Node* context = GetContext();
 
     // Collect the {function} feedback.
-    CollectCallFeedback(function, context, feedback_vector, slot_id);
+    CollectCallOrConstructFeedback(function, context, feedback_vector, slot_id);
 
     Node* result =
         CallJS(function, context, first_arg, args_count, receiver_mode);
@@ -1751,7 +1751,7 @@ class InterpreterJSCallAssembler : public InterpreterAssembler {
     Node* context = GetContext();
 
     // Collect the {function} feedback.
-    CollectCallFeedback(function, context, feedback_vector, slot_id);
+    CollectCallOrConstructFeedback(function, context, feedback_vector, slot_id);
 
     std::array<Node*, Bytecodes::kMaxOperands + kBoilerplateParameterCount>
         temp;