[turbofan] [builtins] Unify construct builtins for JS functions and classes...

[turbofan] [builtins] Unify construct builtins for JS functions and classes and add inlining and deoptimizer support

BUG=v8:6180
R=mstarzinger@chromium.org

Change-Id: Iac5782a0f6b0ff92293421656d907073cfc3f5dd
Reviewed-on: https://chromium-review.googlesource.com/489525
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45232}

src/builtins/builtins-definitions.h

@@ -91,10 +91,9 @@ namespace internal {
   ASM(Construct)                                                               \
   ASM(ConstructWithSpread)                                                     \
   ASM(JSConstructStubApi)                                                      \
-  ASM(JSConstructStubGeneric)                                                  \
+  ASM(JSConstructStubGenericRestrictedReturn)                                  \
+  ASM(JSConstructStubGenericUnrestrictedReturn)                                \
   ASM(JSBuiltinsConstructStub)                                                 \
-  ASM(JSBuiltinsConstructStubForBase)                                          \
-  ASM(JSBuiltinsConstructStubForDerived)                                       \
   TFC(FastNewClosure, FastNewClosure, 1)                                       \
   TFC(FastNewFunctionContextEval, FastNewFunctionContext, 1)                   \
   TFC(FastNewFunctionContextFunction, FastNewFunctionContext, 1)               \

src/builtins/builtins.cc

@@ -249,6 +249,12 @@ bool Builtins::HasCppImplementation(int index) {
 BUILTIN_LIST_ALL(DEFINE_BUILTIN_ACCESSOR)
 #undef DEFINE_BUILTIN_ACCESSOR
 
+Handle<Code> Builtins::JSConstructStubGeneric() {
+  return FLAG_harmony_restrict_constructor_return
+             ? JSConstructStubGenericRestrictedReturn()
+             : JSConstructStubGenericUnrestrictedReturn();
+}
+
 // static
 bool Builtins::AllowDynamicFunction(Isolate* isolate, Handle<JSFunction> target,
                                     Handle<JSObject> target_global_proxy) {

src/builtins/builtins.h

@@ -65,6 +65,7 @@ class Builtins {
   Handle<Code> NewFunctionContext(ScopeType scope_type);
   Handle<Code> NewCloneShallowArray(AllocationSiteMode allocation_mode);
   Handle<Code> NewCloneShallowObject(int length);
+  Handle<Code> JSConstructStubGeneric();
 
   Code* builtin(Name name) {
     // Code::cast cannot be used here since we access builtins

src/compiler/js-inlining.cc

@@ -335,10 +335,11 @@ bool NeedsImplicitReceiver(Handle<SharedFunctionInfo> shared_info) {
   DisallowHeapAllocation no_gc;
   Isolate* const isolate = shared_info->GetIsolate();
   Code* const construct_stub = shared_info->construct_stub();
-  return construct_stub != *isolate->builtins()->JSBuiltinsConstructStub() &&
-         construct_stub !=
-             *isolate->builtins()->JSBuiltinsConstructStubForDerived() &&
-         construct_stub != *isolate->builtins()->JSConstructStubApi();
+  if (construct_stub == *isolate->builtins()->JSConstructStubGeneric()) {
+    return !IsDerivedConstructor(shared_info->kind());
+  } else {
+    return false;
+  }
 }
 
 bool IsNonConstructible(Handle<SharedFunctionInfo> shared_info) {
@@ -486,32 +487,6 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
     return NoChange();
   }
 
-  // TODO(6180): Don't inline class constructors for now, as the
-  // inlining logic doesn't deal properly with class constructors
-  // that return a primitive.
-  if (FLAG_harmony_restrict_constructor_return &&
-      node->opcode() == IrOpcode::kJSConstruct &&
-      IsClassConstructor(shared_info->kind())) {
-    TRACE(
-        "Not inlining %s into %s because class constructor inlining is"
-        "not supported.\n",
-        shared_info->DebugName()->ToCString().get(),
-        info_->shared_info()->DebugName()->ToCString().get());
-    return NoChange();
-  }
-
-  // TODO(706642): Don't inline derived class constructors for now, as the
-  // inlining logic doesn't deal properly with derived class constructors
-  // that return a primitive, i.e. it's not in sync with what the Parser
-  // and the JSConstructSub does.
-  if (node->opcode() == IrOpcode::kJSConstruct &&
-      IsDerivedConstructor(shared_info->kind())) {
-    TRACE("Not inlining %s into %s because constructor is derived.\n",
-          shared_info->DebugName()->ToCString().get(),
-          info_->shared_info()->DebugName()->ToCString().get());
-    return NoChange();
-  }
-
   // Class constructors are callable, but [[Call]] will raise an exception.
   // See ES6 section 9.2.1 [[Call]] ( thisArgument, argumentsList ).
   if (node->opcode() == IrOpcode::kJSCall &&
@@ -669,21 +644,94 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
       uncaught_subcalls.push_back(create);  // Adds {IfSuccess} & {IfException}.
       NodeProperties::ReplaceControlInput(node, create);
       NodeProperties::ReplaceEffectInput(node, create);
+      Node* node_success =
+          NodeProperties::FindSuccessfulControlProjection(node);
+      // Placeholder to hold {node}'s value dependencies while {node} is
+      // replaced.
+      Node* dummy = graph()->NewNode(common()->Dead());
+      NodeProperties::ReplaceUses(node, dummy, node, node, node);
+      Node* result;
+      if (FLAG_harmony_restrict_constructor_return &&
+          IsClassConstructor(shared_info->kind())) {
+        Node* is_undefined =
+            graph()->NewNode(simplified()->ReferenceEqual(), node,
+                             jsgraph()->UndefinedConstant());
+        Node* branch_is_undefined =
+            graph()->NewNode(common()->Branch(), is_undefined, node_success);
+        Node* branch_is_undefined_true =
+            graph()->NewNode(common()->IfTrue(), branch_is_undefined);
+        Node* branch_is_undefined_false =
+            graph()->NewNode(common()->IfFalse(), branch_is_undefined);
+        Node* is_receiver =
+            graph()->NewNode(simplified()->ObjectIsReceiver(), node);
+        Node* branch_is_receiver = graph()->NewNode(
+            common()->Branch(), is_receiver, branch_is_undefined_false);
+        Node* branch_is_receiver_true =
+            graph()->NewNode(common()->IfTrue(), branch_is_receiver);
+        Node* branch_is_receiver_false =
+            graph()->NewNode(common()->IfFalse(), branch_is_receiver);
+        branch_is_receiver_false =
+            graph()->NewNode(javascript()->CallRuntime(
+                                 Runtime::kThrowConstructorReturnedNonObject),
+                             context, NodeProperties::GetFrameStateInput(node),
+                             node, branch_is_receiver_false);
+        uncaught_subcalls.push_back(branch_is_receiver_false);
+        branch_is_receiver_false =
+            graph()->NewNode(common()->Throw(), branch_is_receiver_false,
+                             branch_is_receiver_false);
+        NodeProperties::MergeControlToEnd(graph(), common(),
+                                          branch_is_receiver_false);
+        Node* merge =
+            graph()->NewNode(common()->Merge(2), branch_is_undefined_true,
+                             branch_is_receiver_true);
+        result =
+            graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
+                             create, node, merge);
+        NodeProperties::ReplaceUses(node_success, node_success, node_success,
+                                    merge);
+        // Fix input destroyed by the above {ReplaceUses} call.
+        NodeProperties::ReplaceControlInput(branch_is_undefined, node_success,
+                                            0);
+      } else {
         // Insert a check of the return value to determine whether the return
         // value or the implicit receiver should be selected as a result of the
         // call.
         Node* check = graph()->NewNode(simplified()->ObjectIsReceiver(), node);
-      Node* select =
+        result =
             graph()->NewNode(common()->Select(MachineRepresentation::kTagged),
                              check, node, create);
-      NodeProperties::ReplaceUses(node, select, node, node, node);
-      // Fix-up inputs that have been mangled by the {ReplaceUses} call above.
-      NodeProperties::ReplaceValueInput(select, node, 1);  // Fix-up input.
-      NodeProperties::ReplaceValueInput(check, node, 0);   // Fix-up input.
+      }
       receiver = create;  // The implicit receiver.
+      NodeProperties::ReplaceUses(dummy, result);
+    } else if (IsDerivedConstructor(shared_info->kind())) {
+      Node* node_success =
+          NodeProperties::FindSuccessfulControlProjection(node);
+      Node* is_receiver =
+          graph()->NewNode(simplified()->ObjectIsReceiver(), node);
+      Node* branch_is_receiver =
+          graph()->NewNode(common()->Branch(), is_receiver, node_success);
+      Node* branch_is_receiver_true =
+          graph()->NewNode(common()->IfTrue(), branch_is_receiver);
+      Node* branch_is_receiver_false =
+          graph()->NewNode(common()->IfFalse(), branch_is_receiver);
+      branch_is_receiver_false =
+          graph()->NewNode(javascript()->CallRuntime(
+                               Runtime::kThrowConstructorReturnedNonObject),
+                           context, NodeProperties::GetFrameStateInput(node),
+                           node, branch_is_receiver_false);
+      uncaught_subcalls.push_back(branch_is_receiver_false);
+      branch_is_receiver_false =
+          graph()->NewNode(common()->Throw(), branch_is_receiver_false,
+                           branch_is_receiver_false);
+      NodeProperties::MergeControlToEnd(graph(), common(),
+                                        branch_is_receiver_false);
+
+      NodeProperties::ReplaceUses(node_success, node_success, node_success,
+                                  branch_is_receiver_true);
+      // Fix input destroyed by the above {ReplaceUses} call.
+      NodeProperties::ReplaceControlInput(branch_is_receiver, node_success, 0);
     }
     node->ReplaceInput(1, receiver);
-
     // Insert a construct stub frame into the chain of frame states. This will
     // reconstruct the proper frame when deoptimizing within the constructor.
     frame_state = CreateArtificialFrameState(

src/compiler/node-properties.cc

@@ -138,6 +138,18 @@ bool NodeProperties::IsExceptionalCall(Node* node, Node** out_exception) {
   return false;
 }
 
+// static
+Node* NodeProperties::FindSuccessfulControlProjection(Node* node) {
+  DCHECK_GT(node->op()->ControlOutputCount(), 0);
+  if (node->op()->HasProperty(Operator::kNoThrow)) return node;
+  for (Edge const edge : node->use_edges()) {
+    if (!NodeProperties::IsControlEdge(edge)) continue;
+    if (edge.from()->opcode() == IrOpcode::kIfSuccess) {
+      return edge.from();
+    }
+  }
+  return node;
+}
 
 // static
 void NodeProperties::ReplaceValueInput(Node* node, Node* value, int index) {

src/compiler/node-properties.h

@@ -79,6 +79,10 @@ class V8_EXPORT_PRIVATE NodeProperties final {
   // the present IfException projection is returned via {out_exception}.
   static bool IsExceptionalCall(Node* node, Node** out_exception = nullptr);
 
+  // Returns the node producing the successful control output of {node}. This is
+  // the IfSuccess projection of {node} if present and {node} itself otherwise.
+  static Node* FindSuccessfulControlProjection(Node* node);
+
   // ---------------------------------------------------------------------------
   // Miscellaneous mutators.
 

src/crankshaft/hydrogen.cc

@@ -9538,14 +9538,14 @@ void HOptimizedGraphBuilder::VisitCallNew(CallNew* expr) {
       expr->IsMonomorphic() &&
       IsAllocationInlineable(expr->target())) {
     Handle<JSFunction> constructor = expr->target();
-    DCHECK(
-        constructor->shared()->construct_stub() ==
-            isolate()->builtins()->builtin(Builtins::kJSConstructStubGeneric) ||
-        constructor->shared()->construct_stub() ==
-            isolate()->builtins()->builtin(Builtins::kJSConstructStubApi) ||
+    DCHECK(constructor->shared()->construct_stub() ==
+               isolate()->builtins()->builtin(
+                   Builtins::kJSConstructStubGenericRestrictedReturn) ||
            constructor->shared()->construct_stub() ==
                isolate()->builtins()->builtin(
-                Builtins::kJSBuiltinsConstructStubForBase));
+                   Builtins::kJSConstructStubGenericUnrestrictedReturn) ||
+           constructor->shared()->construct_stub() ==
+               isolate()->builtins()->builtin(Builtins::kJSConstructStubApi));
     HValue* check = Add<HCheckValue>(function, constructor);
 
     // Force completion of inobject slack tracking before generating

src/deoptimizer.cc

@@ -1556,7 +1556,10 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
   int input_index = 0;
 
   Builtins* builtins = isolate_->builtins();
-  Code* construct_stub = builtins->builtin(Builtins::kJSConstructStubGeneric);
+  Code* construct_stub = builtins->builtin(
+      FLAG_harmony_restrict_constructor_return
+          ? Builtins::kJSConstructStubGenericRestrictedReturn
+          : Builtins::kJSConstructStubGenericUnrestrictedReturn);
   BailoutId bailout_id = translated_frame->node_id();
   unsigned height = translated_frame->height();
   unsigned height_in_bytes = height * kPointerSize;
@@ -1662,18 +1665,22 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
     PrintF(trace_scope_->file(), "(%d)\n", height - 1);
   }
 
-  if (bailout_id == BailoutId::ConstructStubCreate()) {
-    // The function was mentioned explicitly in the CONSTRUCT_STUB_FRAME.
+  // The constructor function was mentioned explicitly in the
+  // CONSTRUCT_STUB_FRAME.
   output_offset -= kPointerSize;
   value = reinterpret_cast<intptr_t>(function);
-    WriteValueToOutput(function, 0, frame_index, output_offset, "function ");
-  } else {
-    DCHECK(bailout_id == BailoutId::ConstructStubInvoke());
-    // The newly allocated object was passed as receiver in the artificial
-    // constructor stub environment created by HEnvironment::CopyForInlining().
+  WriteValueToOutput(function, 0, frame_index, output_offset,
+                     "constructor function ");
+
+  // The deopt info contains the implicit receiver or the new target at the
+  // position of the receiver. Copy it to the top of stack.
   output_offset -= kPointerSize;
   value = output_frame->GetFrameSlot(output_frame_size - kPointerSize);
   output_frame->SetFrameSlot(output_offset, value);
+  if (bailout_id == BailoutId::ConstructStubCreate()) {
+    DebugPrintOutputSlot(value, frame_index, output_offset, "new target\n");
+  } else {
+    CHECK(bailout_id == BailoutId::ConstructStubInvoke());
     DebugPrintOutputSlot(value, frame_index, output_offset,
                          "allocated receiver\n");
   }
@@ -1684,8 +1691,7 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
     Register result_reg = FullCodeGenerator::result_register();
     value = input_->GetRegister(result_reg.code());
     output_frame->SetFrameSlot(output_offset, value);
-    DebugPrintOutputSlot(value, frame_index, output_offset,
-                         "constructor result\n");
+    DebugPrintOutputSlot(value, frame_index, output_offset, "subcall result\n");
 
     output_frame->SetState(
         Smi::FromInt(static_cast<int>(BailoutState::TOS_REGISTER)));

src/frames.h

@@ -353,8 +353,10 @@ class ConstructFrameConstants : public TypedFrameConstants {
   // FP-relative.
   static const int kContextOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(0);
   static const int kLengthOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(1);
-  static const int kImplicitReceiverOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(2);
-  DEFINE_TYPED_FRAME_SIZES(3);
+  static const int kConstructorOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(2);
+  static const int kNewTargetOrImplicitReceiverOffset =
+      TYPED_FRAME_PUSHED_VALUE_OFFSET(3);
+  DEFINE_TYPED_FRAME_SIZES(4);
 };
 
 class StubFailureTrampolineFrameConstants : public InternalFrameConstants {

src/heap/heap-inl.h

@@ -791,12 +791,18 @@ void Heap::SetArgumentsAdaptorDeoptPCOffset(int pc_offset) {
 }
 
 void Heap::SetConstructStubCreateDeoptPCOffset(int pc_offset) {
-  DCHECK(construct_stub_create_deopt_pc_offset() == Smi::kZero);
+  // TODO(tebbi): Remove second half of DCHECK once
+  // FLAG_harmony_restrict_constructor_return is gone.
+  DCHECK(construct_stub_create_deopt_pc_offset() == Smi::kZero ||
+         construct_stub_create_deopt_pc_offset() == Smi::FromInt(pc_offset));
   set_construct_stub_create_deopt_pc_offset(Smi::FromInt(pc_offset));
 }
 
 void Heap::SetConstructStubInvokeDeoptPCOffset(int pc_offset) {
-  DCHECK(construct_stub_invoke_deopt_pc_offset() == Smi::kZero);
+  // TODO(tebbi): Remove second half of DCHECK once
+  // FLAG_harmony_restrict_constructor_return is gone.
+  DCHECK(construct_stub_invoke_deopt_pc_offset() == Smi::kZero ||
+         construct_stub_invoke_deopt_pc_offset() == Smi::FromInt(pc_offset));
   set_construct_stub_invoke_deopt_pc_offset(Smi::FromInt(pc_offset));
 }
 

src/objects.h

@@ -6526,6 +6526,10 @@ class SharedFunctionInfo: public HeapObject {
       FunctionKind::kClassConstructor << kCompilerHintsSmiTagSize;
   STATIC_ASSERT(kClassConstructorBitsWithinByte < (1 << kBitsPerByte));
 
+  static const int kDerivedConstructorBitsWithinByte =
+      FunctionKind::kDerivedConstructor << kCompilerHintsSmiTagSize;
+  STATIC_ASSERT(kDerivedConstructorBitsWithinByte < (1 << kBitsPerByte));
+
   static const int kMarkedForTierUpBitWithinByte =
       kMarkedForTierUpBit % kBitsPerByte;
 

src/runtime/runtime-classes.cc

@@ -149,20 +149,6 @@ static MaybeHandle<Object> DefineClass(Isolate* isolate,
   map->SetConstructor(*constructor);
   Handle<JSObject> prototype = isolate->factory()->NewJSObjectFromMap(map);
 
-  if (!super_class->IsTheHole(isolate)) {
-    // Derived classes, just like builtins, don't create implicit receivers in
-    // [[construct]]. Instead they just set up new.target and call into the
-    // constructor. Hence we can reuse the builtins construct stub for derived
-    // classes.
-    Handle<Code> stub =
-        isolate->builtins()->JSBuiltinsConstructStubForDerived();
-    constructor->shared()->SetConstructStub(*stub);
-  } else if (FLAG_harmony_restrict_constructor_return) {
-    DCHECK(super_class->IsTheHole(isolate));
-    Handle<Code> stub = isolate->builtins()->JSBuiltinsConstructStubForBase();
-    constructor->shared()->SetConstructStub(*stub);
-  }
-
   JSFunction::SetPrototype(constructor, prototype);
   PropertyAttributes attribs =
       static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY);

test/mjsunit/compiler/constructor-inlining-no-harmony-restrict-constructor-return.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --no-harmony-restrict-constructor-return --max-deopt-count 200
+
+this.FLAG_harmony_restrict_constructor_return = false;
+try {
+  load('mjsunit/compiler/constructor-inlining.js');
+} catch(e) {
+  load('test/mjsunit/compiler/constructor-inlining.js');
+}

test/mjsunit/compiler/constructor-inlining.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --harmony-restrict-constructor-return --max-deopt-count 200
+
+if (this.FLAG_harmony_restrict_constructor_return === undefined)
+  this.FLAG_harmony_restrict_constructor_return = true;
+var counter = 0;
+var deopt_at = -1;
+
+class Base {
+  constructor(use, x){
+    if (deopt_at-- == 0) {
+        %_DeoptimizeNow();
+        %DeoptimizeFunction(testConstructorInlining);
+    }
+    counter++;
+    this.x = x;
+    if (use) {
+      return x;
+    }
+  }
+}
+
+class Derived extends Base {
+  constructor(use, x, y, deopt = false) {
+    super(use, x);
+    counter++;
+    if (deopt_at-- == 0) %_DeoptimizeNow();
+    this.y = y;
+    if (use) {
+      return y;
+    }
+  }
+}
+
+var DerivedDeoptCreate =  new Proxy(Derived, {
+  get: function(target, name) {
+    if (name=='prototype') {
+      counter++;
+      if (deopt_at-- == 0) %DeoptimizeFunction(Derived);
+    }
+    return target[name];
+  }
+});
+
+function Constr(use, x){
+  counter++;
+  if (deopt_at-- == 0) %_DeoptimizeNow();
+  this.x = x;
+  if (use) {
+    return x;
+  }
+}
+
+%SetForceInlineFlag(Base);
+%SetForceInlineFlag(Derived);
+%SetForceInlineFlag(Constr);
+
+var a = {};
+var b = {};
+
+function testConstructorInlining(){
+  assertEquals(a, new Constr(true, a));
+  assertEquals(7, new Constr(false, 7).x);
+  assertEquals(5, new Constr(true, 5).x);
+
+  assertEquals(a, new Base(true, a));
+  assertEquals(7, new Base(false, 7).x);
+  if (FLAG_harmony_restrict_constructor_return) {
+    // not using assertThrows to ensure proper inlining
+    try {
+      new Base(true, 5);
+      assertTrue(false);
+    } catch (e) {
+      if (!(e instanceof TypeError)) throw e;
+    }
+  } else {
+    assertEquals(5, new Base(true, 5).x);
+  }
+
+  assertEquals(b, new Derived(true, a, b));
+  assertEquals(a, new Derived(true, a, undefined));
+  assertEquals(5, new Derived(false, 5, 7).x);
+  assertEquals(7, new Derived(false, 5, 7).y);
+  try {
+    new Derived(true, a, 7)
+    assertTrue(false);
+  } catch (e) {
+    if (!(e instanceof TypeError)) throw e;
+  }
+  if (FLAG_harmony_restrict_constructor_return) {
+    try {
+      new Derived(true, 5, a)
+      assertTrue(false);
+    } catch (e) {
+      if (!(e instanceof TypeError)) throw e;
+    }
+  } else {
+    assertEquals(a, new Derived(true, 5, a));
+  }
+
+  %OptimizeFunctionOnNextCall(Derived);
+  assertEquals(b, new DerivedDeoptCreate(true, a, b));
+  %OptimizeFunctionOnNextCall(Derived);
+  assertEquals(a, new DerivedDeoptCreate(true, a, undefined));
+  %OptimizeFunctionOnNextCall(Derived);
+  assertEquals(5, new DerivedDeoptCreate(false, 5, 7).x);
+  %OptimizeFunctionOnNextCall(Derived);
+  assertEquals(7, new DerivedDeoptCreate(false, 5, 7).y);
+}
+
+testConstructorInlining();
+%OptimizeFunctionOnNextCall(testConstructorInlining);
+testConstructorInlining();
+
+var last = undefined;
+for(var i = 0; deopt_at < 0; ++i) {
+  deopt_at = i;
+  counter = 0;
+  %OptimizeFunctionOnNextCall(testConstructorInlining);
+  testConstructorInlining();
+  if (last !== undefined) {
+    assertEquals(counter, last)
+  }
+  last = counter;
+}