Reviewers: adamk, arv,

Message:
PTAL, ia32 only for now.

Description:
Do not add extra argument for new.target

JSConstructStub for subclass constructors instead locates new.target in
a known location on the stack.

R=a...@chromium.org,ad...@chromium.org
BUG=v8:3886
LOG=N

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

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

Affected files (+34, -55 lines):
  M src/code-stubs.h
  M src/ia32/builtins-ia32.cc
  M src/ia32/code-stubs-ia32.cc
  M src/ia32/full-codegen-ia32.cc
  M src/objects.cc


Index: src/code-stubs.h
diff --git a/src/code-stubs.h b/src/code-stubs.h
index 515f2e578e613470e9a228826df201f7bba6d116..e6a3e3a2b9feb48a658b86338e8496c5a8d7959e 100644
--- a/src/code-stubs.h
+++ b/src/code-stubs.h
@@ -1785,13 +1785,8 @@ class ArgumentsAccessStub: public PlatformCodeStub {
     NEW_STRICT
   };

-  enum HasNewTarget { NO_NEW_TARGET, HAS_NEW_TARGET };
-
-  ArgumentsAccessStub(Isolate* isolate, Type type,
-                      HasNewTarget has_new_target = NO_NEW_TARGET)
-      : PlatformCodeStub(isolate) {
-    minor_key_ =
-        TypeBits::encode(type) | HasNewTargetBits::encode(has_new_target);
+ ArgumentsAccessStub(Isolate* isolate, Type type) : PlatformCodeStub(isolate) {
+    minor_key_ = TypeBits::encode(type);
   }

   CallInterfaceDescriptor GetCallInterfaceDescriptor() override {
@@ -1803,9 +1798,6 @@ class ArgumentsAccessStub: public PlatformCodeStub {

  private:
   Type type() const { return TypeBits::decode(minor_key_); }
-  bool has_new_target() const {
-    return HasNewTargetBits::decode(minor_key_) == HAS_NEW_TARGET;
-  }

   void GenerateReadElement(MacroAssembler* masm);
   void GenerateNewStrict(MacroAssembler* masm);
@@ -1815,7 +1807,6 @@ class ArgumentsAccessStub: public PlatformCodeStub {
   void PrintName(std::ostream& os) const override;  // NOLINT

   class TypeBits : public BitField<Type, 0, 2> {};
-  class HasNewTargetBits : public BitField<HasNewTarget, 2, 1> {};

   DEFINE_PLATFORM_CODE_STUB(ArgumentsAccess, PlatformCodeStub);
 };
Index: src/ia32/builtins-ia32.cc
diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc
index 429be9227aa907b88ee4aa2fdf17590486872084..da7ab51204200895c87bf797a25958949a7bc3d6 100644
--- a/src/ia32/builtins-ia32.cc
+++ b/src/ia32/builtins-ia32.cc
@@ -530,9 +530,6 @@ void Builtins::Generate_JSConstructStubForDerived(MacroAssembler* masm) {
     __ dec(ecx);
     __ j(greater_equal, &loop);

-    __ inc(eax);  // Pushed new.target.
-
-
     // Handle step in.
     Label skip_step_in;
     ExternalReference debug_step_in_fp =
@@ -556,7 +553,7 @@ void Builtins::Generate_JSConstructStubForDerived(MacroAssembler* masm) {
     // Restore context from the frame.
     __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));

-    __ mov(ebx, Operand(esp, 0));
+    __ mov(ebx, Operand(esp, kPointerSize));
   }

   __ pop(ecx);  // Return address.
Index: src/ia32/code-stubs-ia32.cc
diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index 7435ccd8aa12f7bce975dbd2423ebc4edec4138a..1d53d719a99b0fc1a7eb539ef4d4e16f19e65543 100644
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -733,7 +733,6 @@ void LoadIndexedStringStub::Generate(MacroAssembler* masm) {


 void ArgumentsAccessStub::GenerateReadElement(MacroAssembler* masm) {
-  CHECK(!has_new_target());
   // The key is in edx and the parameter count is in eax.
   DCHECK(edx.is(ArgumentsAccessReadDescriptor::index()));
   DCHECK(eax.is(ArgumentsAccessReadDescriptor::parameter_count()));
@@ -800,8 +799,6 @@ void ArgumentsAccessStub::GenerateNewSloppySlow(MacroAssembler* masm) {
   // esp[8] : receiver displacement
   // esp[12] : function

-  CHECK(!has_new_target());
-
   // Check if the calling frame is an arguments adaptor frame.
   Label runtime;
   __ mov(edx, Operand(ebp, StandardFrameConstants::kCallerFPOffset));
@@ -830,8 +827,6 @@ void ArgumentsAccessStub::GenerateNewSloppyFast(MacroAssembler* masm) {
   // ebx = parameter count (tagged)
   __ mov(ebx, Operand(esp, 1 * kPointerSize));

-  CHECK(!has_new_target());
-
   // Check if the calling frame is an arguments adaptor frame.
// TODO(rossberg): Factor out some of the bits that are shared with the other
   // Generate* functions.
@@ -1072,18 +1067,6 @@ void ArgumentsAccessStub::GenerateNewStrict(MacroAssembler* masm) {
   __ bind(&adaptor_frame);
   __ mov(ecx, Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset));

-  if (has_new_target()) {
- // If the constructor was [[Call]]ed, the call will not push a new.target - // onto the stack. In that case the arguments array we construct is bogus,
-    // bu we do not care as the constructor throws immediately.
-    __ cmp(ecx, Immediate(Smi::FromInt(0)));
-    Label skip_decrement;
-    __ j(equal, &skip_decrement);
-    // Subtract 1 from smi-tagged arguments count.
-    __ sub(ecx, Immediate(2));
-    __ bind(&skip_decrement);
-  }
-
   __ lea(edx, Operand(edx, ecx, times_2,
                       StandardFrameConstants::kCallerSPOffset));
   __ mov(Operand(esp, 1 * kPointerSize), ecx);
Index: src/ia32/full-codegen-ia32.cc
diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index 1c7b75443f80bef4bd9c8baeb334fdbe7fb5bc85..0a26f40ccbd06c8e55c23f3e4f9f3129960234e3 100644
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -246,16 +246,37 @@ void FullCodeGenerator::Generate() {
   if (new_target_var != nullptr) {
     Comment cmnt(masm_, "[ new.target");
     // new.target is parameter -2.
-    int offset = 2 * kPointerSize + kFPOnStackSize + kPCOnStackSize +
-                 (info_->scope()->num_parameters() - 1) * kPointerSize;
-    __ mov(eax, Operand(ebp, offset));
+    // int offset = 2 * kPointerSize + kFPOnStackSize + kPCOnStackSize +
+    //             (info_->scope()->num_parameters() - 1) * kPointerSize;
+    __ mov(eax, Operand(ebp, StandardFrameConstants::kCallerFPOffset));
+
+    Label non_adaptor_frame;
+    __ cmp(Operand(eax, StandardFrameConstants::kContextOffset),
+           Immediate(Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR)));
+    __ j(not_equal, &non_adaptor_frame);
+    __ mov(eax, Operand(eax, StandardFrameConstants::kCallerFPOffset));
+
+    __ bind(&non_adaptor_frame);
+    __ cmp(Operand(eax, StandardFrameConstants::kMarkerOffset),
+           Immediate(Smi::FromInt(StackFrame::CONSTRUCT)));
+
+    Label non_construct_frame, done;
+    __ j(not_equal, &non_construct_frame);
+
+    // Construct frame
+    __ mov(eax, Operand(eax, StandardFrameConstants::kExpressionsOffset -
+                                 2 * kPointerSize));
     SetVar(new_target_var, eax, ebx, edx);
+    __ jmp(&done);
+
+    // Non-construct frame
+    __ bind(&non_construct_frame);
+    __ mov(eax, Immediate(isolate()->factory()->undefined_value()));
+    SetVar(new_target_var, eax, ebx, edx);
+
+    __ bind(&done);
   }

-  ArgumentsAccessStub::HasNewTarget has_new_target =
-      IsSubclassConstructor(info->function()->kind())
-          ? ArgumentsAccessStub::HAS_NEW_TARGET
-          : ArgumentsAccessStub::NO_NEW_TARGET;

   // Possibly allocate RestParameters
   int rest_index;
@@ -265,10 +286,6 @@ void FullCodeGenerator::Generate() {

     int num_parameters = info->scope()->num_parameters();
     int offset = num_parameters * kPointerSize;
-    if (has_new_target == ArgumentsAccessStub::HAS_NEW_TARGET) {
-      --num_parameters;
-      ++rest_index;
-    }

     __ lea(edx,
            Operand(ebp, StandardFrameConstants::kCallerSPOffset + offset));
@@ -312,7 +329,7 @@ void FullCodeGenerator::Generate() {
       type = ArgumentsAccessStub::NEW_SLOPPY_FAST;
     }

-    ArgumentsAccessStub stub(isolate(), type, has_new_target);
+    ArgumentsAccessStub stub(isolate(), type);
     __ CallStub(&stub);

     SetVar(arguments, eax, ebx, edx);
@@ -462,9 +479,6 @@ void FullCodeGenerator::EmitReturnSequence() {
     __ pop(ebp);

     int arg_count = info_->scope()->num_parameters() + 1;
-    if (IsSubclassConstructor(info_->function()->kind())) {
-      arg_count++;
-    }
     int arguments_bytes = arg_count * kPointerSize;
     __ Ret(arguments_bytes, ecx);
     // Check that the size of the code used for returning is large enough
@@ -3016,6 +3030,7 @@ void FullCodeGenerator::EmitInitializeThisAfterSuper(
   Variable* this_var = super_call_ref->this_var()->var();
   GetVar(ecx, this_var);
   __ cmp(ecx, isolate()->factory()->the_hole_value());
+
   Label uninitialized_this;
   __ j(equal, &uninitialized_this);
   __ push(Immediate(this_var->name()));
@@ -4180,8 +4195,6 @@ void FullCodeGenerator::EmitDefaultConstructorCallSuper(CallRuntime* expr) { __ mov(ecx, Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset));
     __ SmiUntag(ecx);

-    // Subtract 1 from arguments count, for new.target.
-    __ sub(ecx, Immediate(1));
     __ mov(eax, ecx);
     __ lea(edx, Operand(edx, ecx, times_pointer_size,
                         StandardFrameConstants::kCallerSPOffset));
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index f2d617b8298e93d45d9dfb580f8549a7bfbfb0fa..86fee8ed50b08f5aa1c038abe50e5112d984540e 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -10616,12 +10616,7 @@ void SharedFunctionInfo::DisableOptimization(BailoutReason reason) {
 void SharedFunctionInfo::InitFromFunctionLiteral(
     Handle<SharedFunctionInfo> shared_info, FunctionLiteral* lit) {
   shared_info->set_length(lit->scope()->default_function_length());
-  if (IsSubclassConstructor(lit->kind())) {
- shared_info->set_internal_formal_parameter_count(lit->parameter_count() +
-                                                     1);
-  } else {
- shared_info->set_internal_formal_parameter_count(lit->parameter_count());
-  }
+  shared_info->set_internal_formal_parameter_count(lit->parameter_count());
   shared_info->set_function_token_position(lit->function_token_position());
   shared_info->set_start_position(lit->start_position());
   shared_info->set_end_position(lit->end_position());


--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to