[calls] Consistent call protocol for calls.
authorbmeurer <bmeurer@chromium.org>
Wed, 9 Sep 2015 05:01:01 +0000 (22:01 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 9 Sep 2015 05:01:22 +0000 (05:01 +0000)
The number of actual arguments should always be available, there's no
point in trying to optimize away a simple assignment of an immediate to
a register before some calls.

The main motivation is to have a consistent state at the beginning of every
function. Currently the arguments register (i.e. rax or eax) either contains
the number of arguments or some random garbage depending on whether
the callsite decided that the callee might need the information or not.
This causes trouble with runtime implementations of functions that
do not set internal_formal_parameter_count to the DontAdaptArguments
sentinel (we don't have any of those yet), but also makes it impossible
to sanity check the arguments in the callee, because the callee doesn't
know whether the caller decided to pass the number of arguments or
random garbage.

BUG=v8:4413
LOG=n

Review URL: https://codereview.chromium.org/1330033002

Cr-Commit-Position: refs/heads/master@{#30648}

16 files changed:
src/arm/lithium-codegen-arm.cc
src/arm/macro-assembler-arm.cc
src/arm64/lithium-codegen-arm64.cc
src/arm64/macro-assembler-arm64.cc
src/hydrogen-instructions.cc
src/hydrogen-instructions.h
src/hydrogen.cc
src/hydrogen.h
src/ia32/lithium-codegen-ia32.cc
src/ia32/macro-assembler-ia32.cc
src/mips/lithium-codegen-mips.cc
src/mips/macro-assembler-mips.cc
src/mips64/lithium-codegen-mips64.cc
src/mips64/macro-assembler-mips64.cc
src/x64/lithium-codegen-x64.cc
src/x64/macro-assembler-x64.cc

index 6e368de036c72366b0f11e5ff6bee38b2233a562..74f527c4a7cf91cf3944113e5e87aba843627aa0 100644 (file)
@@ -3464,11 +3464,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
     // Change context.
     __ ldr(cp, FieldMemOperand(function_reg, JSFunction::kContextOffset));
 
-    // Set r0 to arguments count if adaption is not needed. Assumes that r0
-    // is available to write to at this point.
-    if (dont_adapt_arguments) {
-      __ mov(r0, Operand(arity));
-    }
+    // Always initialize r0 to the number of actual arguments.
+    __ mov(r0, Operand(arity));
 
     // Invoke function.
     __ ldr(ip, FieldMemOperand(function_reg, JSFunction::kCodeEntryOffset));
@@ -3849,9 +3846,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
   DCHECK(ToRegister(instr->function()).is(r1));
   DCHECK(ToRegister(instr->result()).is(r0));
 
-  if (instr->hydrogen()->pass_argument_count()) {
-    __ mov(r0, Operand(instr->arity()));
-  }
+  __ mov(r0, Operand(instr->arity()));
 
   // Change context.
   __ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset));
index 3f0ff1aa11da999f622eee4e83d2e6fa2a3e8180..c0b9773ad8e895d1f51f4c480e5fb5b341d5540d 100644 (file)
@@ -1251,10 +1251,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
 
   if (expected.is_immediate()) {
     DCHECK(actual.is_immediate());
+    mov(r0, Operand(actual.immediate()));
     if (expected.immediate() == actual.immediate()) {
       definitely_matches = true;
     } else {
-      mov(r0, Operand(actual.immediate()));
       const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
       if (expected.immediate() == sentinel) {
         // Don't worry about adapting arguments for builtins that
@@ -1269,9 +1269,9 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
     }
   } else {
     if (actual.is_immediate()) {
+      mov(r0, Operand(actual.immediate()));
       cmp(expected.reg(), Operand(actual.immediate()));
       b(eq, &regular_invoke);
-      mov(r0, Operand(actual.immediate()));
     } else {
       cmp(expected.reg(), Operand(actual.reg()));
       b(eq, &regular_invoke);
index 529352d77e621eee59eab594d09ac1c1ea55821f..fe16d06d4348d1d2db3dd37f08e3e01220878027 100644 (file)
@@ -1996,11 +1996,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
     // Change context.
     __ Ldr(cp, FieldMemOperand(function_reg, JSFunction::kContextOffset));
 
-    // Set the arguments count if adaption is not needed. Assumes that x0 is
-    // available to write to at this point.
-    if (dont_adapt_arguments) {
-      __ Mov(arity_reg, arity);
-    }
+    // Always initialize x0 to the number of actual arguments.
+    __ Mov(arity_reg, arity);
 
     // Invoke function.
     __ Ldr(x10, FieldMemOperand(function_reg, JSFunction::kCodeEntryOffset));
@@ -2067,9 +2064,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
   DCHECK(instr->IsMarkedAsCall());
   DCHECK(ToRegister(instr->function()).is(x1));
 
-  if (instr->hydrogen()->pass_argument_count()) {
-    __ Mov(x0, Operand(instr->arity()));
-  }
+  __ Mov(x0, Operand(instr->arity()));
 
   // Change context.
   __ Ldr(cp, FieldMemOperand(x1, JSFunction::kContextOffset));
index 75814e83a866396c66b89cb601fa2a02e6bf16d1..b4870df9e87a24a770f4811c74d5dfc7ec543d12 100644 (file)
@@ -2571,11 +2571,11 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
 
   if (expected.is_immediate()) {
     DCHECK(actual.is_immediate());
+    Mov(x0, actual.immediate());
     if (expected.immediate() == actual.immediate()) {
       definitely_matches = true;
 
     } else {
-      Mov(x0, actual.immediate());
       if (expected.immediate() ==
           SharedFunctionInfo::kDontAdaptArgumentsSentinel) {
         // Don't worry about adapting arguments for builtins that
@@ -2593,11 +2593,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
   } else {  // expected is a register.
     Operand actual_op = actual.is_immediate() ? Operand(actual.immediate())
                                               : Operand(actual.reg());
+    Mov(x0, actual_op);
     // If actual == expected perform a regular invocation.
     Cmp(expected.reg(), actual_op);
     B(eq, &regular_invoke);
-    // Otherwise set up x0 for the argument adaptor.
-    Mov(x0, actual_op);
   }
 
   // If the argument counts may mismatch, generate a call to the argument
index be0b71be41b750158b1e3c56d3c42da75b95aa5a..e9ca099225cf83804076fe22ca8eaa60ab92ab44 100644 (file)
@@ -927,8 +927,7 @@ std::ostream& HCallJSFunction::PrintDataTo(std::ostream& os) const {  // NOLINT
 
 HCallJSFunction* HCallJSFunction::New(Isolate* isolate, Zone* zone,
                                       HValue* context, HValue* function,
-                                      int argument_count,
-                                      bool pass_argument_count) {
+                                      int argument_count) {
   bool has_stack_check = false;
   if (function->IsConstant()) {
     HConstant* fun_const = HConstant::cast(function);
@@ -939,9 +938,7 @@ HCallJSFunction* HCallJSFunction::New(Isolate* isolate, Zone* zone,
          jsfun->code()->kind() == Code::OPTIMIZED_FUNCTION);
   }
 
-  return new(zone) HCallJSFunction(
-      function, argument_count, pass_argument_count,
-      has_stack_check);
+  return new (zone) HCallJSFunction(function, argument_count, has_stack_check);
 }
 
 
index e1bca999e2290a9ff0ecaa17c5bd69b68519c642..9ff807968d1f530e9c9c466d4d6ed7fbae3a27ef 100644 (file)
@@ -2228,8 +2228,7 @@ class HBinaryCall : public HCall<2> {
 class HCallJSFunction final : public HCall<1> {
  public:
   static HCallJSFunction* New(Isolate* isolate, Zone* zone, HValue* context,
-                              HValue* function, int argument_count,
-                              bool pass_argument_count);
+                              HValue* function, int argument_count);
 
   HValue* function() const { return OperandAt(0); }
 
@@ -2240,8 +2239,6 @@ class HCallJSFunction final : public HCall<1> {
     return Representation::Tagged();
   }
 
-  bool pass_argument_count() const { return pass_argument_count_; }
-
   bool HasStackCheck() final { return has_stack_check_; }
 
   DECLARE_CONCRETE_INSTRUCTION(CallJSFunction)
@@ -2250,15 +2247,12 @@ class HCallJSFunction final : public HCall<1> {
   // The argument count includes the receiver.
   HCallJSFunction(HValue* function,
                   int argument_count,
-                  bool pass_argument_count,
                   bool has_stack_check)
       : HCall<1>(argument_count),
-        pass_argument_count_(pass_argument_count),
         has_stack_check_(has_stack_check) {
       SetOperandAt(0, function);
   }
 
-  bool pass_argument_count_;
   bool has_stack_check_;
 };
 
index 40d4c83e88a6a0419082336228d3375bad3f78c3..bf55f7ca66e3eb65845a852d565769716950c53a 100644 (file)
@@ -7901,9 +7901,9 @@ void HOptimizedGraphBuilder::AddCheckPrototypeMaps(Handle<JSObject> holder,
 }
 
 
-HInstruction* HOptimizedGraphBuilder::NewPlainFunctionCall(
-    HValue* fun, int argument_count, bool pass_argument_count) {
-  return New<HCallJSFunction>(fun, argument_count, pass_argument_count);
+HInstruction* HOptimizedGraphBuilder::NewPlainFunctionCall(HValue* fun,
+                                                           int argument_count) {
+  return New<HCallJSFunction>(fun, argument_count);
 }
 
 
@@ -7941,7 +7941,7 @@ HInstruction* HOptimizedGraphBuilder::BuildCallConstantFunction(
     if (jsfun.is_identical_to(current_info()->closure())) {
       graph()->MarkRecursive();
     }
-    return NewPlainFunctionCall(target, argument_count, dont_adapt_arguments);
+    return NewPlainFunctionCall(target, argument_count);
   } else {
     HValue* param_count_value = Add<HConstant>(formal_parameter_count);
     HValue* context = Add<HLoadNamedField>(
@@ -8962,7 +8962,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
           if_inline.Else();
           {
             Add<HPushArguments>(receiver);
-            result = Add<HCallJSFunction>(function, 1, true);
+            result = Add<HCallJSFunction>(function, 1);
             if (!ast_context()->IsEffect()) Push(result);
           }
           if_inline.End();
index 88a9fd81c0520cb8dfaa9c54e0c02e40f2603a4d..f60f2cb726b6d5dd1fd3fe9c2ee78b6a54d94be1 100644 (file)
@@ -2865,9 +2865,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   void AddCheckPrototypeMaps(Handle<JSObject> holder,
                              Handle<Map> receiver_map);
 
-  HInstruction* NewPlainFunctionCall(HValue* fun,
-                                     int argument_count,
-                                     bool pass_argument_count);
+  HInstruction* NewPlainFunctionCall(HValue* fun, int argument_count);
 
   HInstruction* NewArgumentAdaptorCall(HValue* fun, HValue* context,
                                        int argument_count,
index 7f44b9e77eda404aa4d9206821d1fa0ffee950a9..24f6f427cb7d44fa412846b456fc47380acf820f 100644 (file)
@@ -3340,11 +3340,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
     // Change context.
     __ mov(esi, FieldOperand(function_reg, JSFunction::kContextOffset));
 
-    // Set eax to arguments count if adaption is not needed. Assumes that eax
-    // is available to write to at this point.
-    if (dont_adapt_arguments) {
-      __ mov(eax, arity);
-    }
+    // Always initialize eax to the number of actual arguments.
+    __ mov(eax, arity);
 
     // Invoke function directly.
     if (function.is_identical_to(info()->closure())) {
@@ -3406,9 +3403,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
   DCHECK(ToRegister(instr->function()).is(edi));
   DCHECK(ToRegister(instr->result()).is(eax));
 
-  if (instr->hydrogen()->pass_argument_count()) {
-    __ mov(eax, instr->arity());
-  }
+  __ mov(eax, instr->arity());
 
   // Change context.
   __ mov(esi, FieldOperand(edi, JSFunction::kContextOffset));
index 96c81609be937fb1fda59a51dbccb9ec837e5a38..474bb7773236aa1ff1690c17e30a0780271153ae 100644 (file)
@@ -1923,10 +1923,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
   Label invoke;
   if (expected.is_immediate()) {
     DCHECK(actual.is_immediate());
+    mov(eax, actual.immediate());
     if (expected.immediate() == actual.immediate()) {
       definitely_matches = true;
     } else {
-      mov(eax, actual.immediate());
       const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
       if (expected.immediate() == sentinel) {
         // Don't worry about adapting arguments for builtins that
@@ -1944,10 +1944,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       // Expected is in register, actual is immediate. This is the
       // case when we invoke function values without going through the
       // IC mechanism.
+      mov(eax, actual.immediate());
       cmp(expected.reg(), actual.immediate());
       j(equal, &invoke);
       DCHECK(expected.reg().is(ebx));
-      mov(eax, actual.immediate());
     } else if (!expected.reg().is(actual.reg())) {
       // Both expected and actual are in (different) registers. This
       // is the case when we invoke functions using call and apply.
@@ -1955,6 +1955,8 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       j(equal, &invoke);
       DCHECK(actual.reg().is(eax));
       DCHECK(expected.reg().is(ebx));
+    } else {
+      Move(eax, actual.reg());
     }
   }
 
index 5586ed1ab264e7dd1f1a68ed40a313ff71e370e3..bbe509213c9bca6ccc4be107fb34383cc8356e63 100644 (file)
@@ -3413,11 +3413,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
     // Change context.
     __ lw(cp, FieldMemOperand(function_reg, JSFunction::kContextOffset));
 
-    // Set r0 to arguments count if adaption is not needed. Assumes that r0
-    // is available to write to at this point.
-    if (dont_adapt_arguments) {
-      __ li(a0, Operand(arity));
-    }
+    // Always initialize a0 to the number of actual arguments.
+    __ li(a0, Operand(arity));
 
     // Invoke function.
     __ lw(at, FieldMemOperand(function_reg, JSFunction::kCodeEntryOffset));
@@ -3824,9 +3821,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
   DCHECK(ToRegister(instr->function()).is(a1));
   DCHECK(ToRegister(instr->result()).is(v0));
 
-  if (instr->hydrogen()->pass_argument_count()) {
-    __ li(a0, Operand(instr->arity()));
-  }
+  __ li(a0, Operand(instr->arity()));
 
   // Change context.
   __ lw(cp, FieldMemOperand(a1, JSFunction::kContextOffset));
index 1e175f46495a5c0cb2d7a4a87115549d69089399..eb33c235a816ddb827094fdf5c4a04239961f33f 100644 (file)
@@ -4081,10 +4081,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
 
   if (expected.is_immediate()) {
     DCHECK(actual.is_immediate());
+    li(a0, Operand(actual.immediate()));
     if (expected.immediate() == actual.immediate()) {
       definitely_matches = true;
     } else {
-      li(a0, Operand(actual.immediate()));
       const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
       if (expected.immediate() == sentinel) {
         // Don't worry about adapting arguments for builtins that
@@ -4098,8 +4098,8 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       }
     }
   } else if (actual.is_immediate()) {
-    Branch(&regular_invoke, eq, expected.reg(), Operand(actual.immediate()));
     li(a0, Operand(actual.immediate()));
+    Branch(&regular_invoke, eq, expected.reg(), Operand(a0));
   } else {
     Branch(&regular_invoke, eq, expected.reg(), Operand(actual.reg()));
   }
index a612681c5b37086779db837e4ec22f94da26eecd..4eeb53b53848c8203eb352f1a4e56dc9cdca5b12 100644 (file)
@@ -3582,11 +3582,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
     // Change context.
     __ ld(cp, FieldMemOperand(function_reg, JSFunction::kContextOffset));
 
-    // Set r0 to arguments count if adaption is not needed. Assumes that r0
-    // is available to write to at this point.
-    if (dont_adapt_arguments) {
-      __ li(a0, Operand(arity));
-    }
+    // Always initialize a0 to the number of actual arguments.
+    __ li(a0, Operand(arity));
 
     // Invoke function.
     __ ld(at, FieldMemOperand(function_reg, JSFunction::kCodeEntryOffset));
@@ -4012,9 +4009,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
   DCHECK(ToRegister(instr->function()).is(a1));
   DCHECK(ToRegister(instr->result()).is(v0));
 
-  if (instr->hydrogen()->pass_argument_count()) {
-    __ li(a0, Operand(instr->arity()));
-  }
+  __ li(a0, Operand(instr->arity()));
 
   // Change context.
   __ ld(cp, FieldMemOperand(a1, JSFunction::kContextOffset));
index 0568cd6bea8b5c82b8a5ec724118b7c9c3b4c8ee..0aa67852787c212df27800cd2b580984eb33db76 100644 (file)
@@ -4084,10 +4084,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
 
   if (expected.is_immediate()) {
     DCHECK(actual.is_immediate());
+    li(a0, Operand(actual.immediate()));
     if (expected.immediate() == actual.immediate()) {
       definitely_matches = true;
     } else {
-      li(a0, Operand(actual.immediate()));
       const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
       if (expected.immediate() == sentinel) {
         // Don't worry about adapting arguments for builtins that
@@ -4101,8 +4101,8 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       }
     }
   } else if (actual.is_immediate()) {
-    Branch(&regular_invoke, eq, expected.reg(), Operand(actual.immediate()));
     li(a0, Operand(actual.immediate()));
+    Branch(&regular_invoke, eq, expected.reg(), Operand(a0));
   } else {
     Branch(&regular_invoke, eq, expected.reg(), Operand(actual.reg()));
   }
index 250589097452890ef6631493e43d4ccf17269d27..4557ce13be052dbf5af806962863908581899e6a 100644 (file)
@@ -3411,11 +3411,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
     // Change context.
     __ movp(rsi, FieldOperand(function_reg, JSFunction::kContextOffset));
 
-    // Set rax to arguments count if adaption is not needed. Assumes that rax
-    // is available to write to at this point.
-    if (dont_adapt_arguments) {
-      __ Set(rax, arity);
-    }
+    // Always initialize rax to the number of actual arguments.
+    __ Set(rax, arity);
 
     // Invoke function.
     if (function.is_identical_to(info()->closure())) {
@@ -3478,9 +3475,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
   DCHECK(ToRegister(instr->function()).is(rdi));
   DCHECK(ToRegister(instr->result()).is(rax));
 
-  if (instr->hydrogen()->pass_argument_count()) {
-    __ Set(rax, instr->arity());
-  }
+  __ Set(rax, instr->arity());
 
   // Change context.
   __ movp(rsi, FieldOperand(rdi, JSFunction::kContextOffset));
index 884cbab0efb62ad660e1fb749566406b2ff595f4..e3ab7c7e3d30361e4074893025414db87c229767 100644 (file)
@@ -3632,10 +3632,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
   Label invoke;
   if (expected.is_immediate()) {
     DCHECK(actual.is_immediate());
+    Set(rax, actual.immediate());
     if (expected.immediate() == actual.immediate()) {
       definitely_matches = true;
     } else {
-      Set(rax, actual.immediate());
       if (expected.immediate() ==
               SharedFunctionInfo::kDontAdaptArgumentsSentinel) {
         // Don't worry about adapting arguments for built-ins that
@@ -3653,10 +3653,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       // Expected is in register, actual is immediate. This is the
       // case when we invoke function values without going through the
       // IC mechanism.
+      Set(rax, actual.immediate());
       cmpp(expected.reg(), Immediate(actual.immediate()));
       j(equal, &invoke, Label::kNear);
       DCHECK(expected.reg().is(rbx));
-      Set(rax, actual.immediate());
     } else if (!expected.reg().is(actual.reg())) {
       // Both expected and actual are in (different) registers. This
       // is the case when we invoke functions using call and apply.
@@ -3664,6 +3664,8 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
       j(equal, &invoke, Label::kNear);
       DCHECK(actual.reg().is(rax));
       DCHECK(expected.reg().is(rbx));
+    } else {
+      Move(rax, actual.reg());
     }
   }