From: bmeurer Date: Wed, 9 Sep 2015 05:01:01 +0000 (-0700) Subject: [calls] Consistent call protocol for calls. X-Git-Tag: upstream/4.7.83~392 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b37907ff7f866873ddfbfc97670b43c19a5fc7f9;p=platform%2Fupstream%2Fv8.git [calls] Consistent call protocol for calls. 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} --- diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 6e368de03..74f527c4a 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -3464,11 +3464,8 @@ void LCodeGen::CallKnownFunction(Handle 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)); diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index 3f0ff1aa1..c0b9773ad 100644 --- a/src/arm/macro-assembler-arm.cc +++ b/src/arm/macro-assembler-arm.cc @@ -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, ®ular_invoke); - mov(r0, Operand(actual.immediate())); } else { cmp(expected.reg(), Operand(actual.reg())); b(eq, ®ular_invoke); diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index 529352d77..fe16d06d4 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -1996,11 +1996,8 @@ void LCodeGen::CallKnownFunction(Handle 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)); diff --git a/src/arm64/macro-assembler-arm64.cc b/src/arm64/macro-assembler-arm64.cc index 75814e83a..b4870df9e 100644 --- a/src/arm64/macro-assembler-arm64.cc +++ b/src/arm64/macro-assembler-arm64.cc @@ -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, ®ular_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 diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index be0b71be4..e9ca09922 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -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); } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index e1bca999e..9ff807968 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -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_; }; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 40d4c83e8..bf55f7ca6 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -7901,9 +7901,9 @@ void HOptimizedGraphBuilder::AddCheckPrototypeMaps(Handle holder, } -HInstruction* HOptimizedGraphBuilder::NewPlainFunctionCall( - HValue* fun, int argument_count, bool pass_argument_count) { - return New(fun, argument_count, pass_argument_count); +HInstruction* HOptimizedGraphBuilder::NewPlainFunctionCall(HValue* fun, + int argument_count) { + return New(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(formal_parameter_count); HValue* context = Add( @@ -8962,7 +8962,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall( if_inline.Else(); { Add(receiver); - result = Add(function, 1, true); + result = Add(function, 1); if (!ast_context()->IsEffect()) Push(result); } if_inline.End(); diff --git a/src/hydrogen.h b/src/hydrogen.h index 88a9fd81c..f60f2cb72 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -2865,9 +2865,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { void AddCheckPrototypeMaps(Handle holder, Handle 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, diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 7f44b9e77..24f6f427c 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -3340,11 +3340,8 @@ void LCodeGen::CallKnownFunction(Handle 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)); diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index 96c81609b..474bb7773 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -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()); } } diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index 5586ed1ab..bbe509213 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -3413,11 +3413,8 @@ void LCodeGen::CallKnownFunction(Handle 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)); diff --git a/src/mips/macro-assembler-mips.cc b/src/mips/macro-assembler-mips.cc index 1e175f464..eb33c235a 100644 --- a/src/mips/macro-assembler-mips.cc +++ b/src/mips/macro-assembler-mips.cc @@ -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(®ular_invoke, eq, expected.reg(), Operand(actual.immediate())); li(a0, Operand(actual.immediate())); + Branch(®ular_invoke, eq, expected.reg(), Operand(a0)); } else { Branch(®ular_invoke, eq, expected.reg(), Operand(actual.reg())); } diff --git a/src/mips64/lithium-codegen-mips64.cc b/src/mips64/lithium-codegen-mips64.cc index a612681c5..4eeb53b53 100644 --- a/src/mips64/lithium-codegen-mips64.cc +++ b/src/mips64/lithium-codegen-mips64.cc @@ -3582,11 +3582,8 @@ void LCodeGen::CallKnownFunction(Handle 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)); diff --git a/src/mips64/macro-assembler-mips64.cc b/src/mips64/macro-assembler-mips64.cc index 0568cd6be..0aa678527 100644 --- a/src/mips64/macro-assembler-mips64.cc +++ b/src/mips64/macro-assembler-mips64.cc @@ -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(®ular_invoke, eq, expected.reg(), Operand(actual.immediate())); li(a0, Operand(actual.immediate())); + Branch(®ular_invoke, eq, expected.reg(), Operand(a0)); } else { Branch(®ular_invoke, eq, expected.reg(), Operand(actual.reg())); } diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 250589097..4557ce13b 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -3411,11 +3411,8 @@ void LCodeGen::CallKnownFunction(Handle 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)); diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc index 884cbab0e..e3ab7c7e3 100644 --- a/src/x64/macro-assembler-x64.cc +++ b/src/x64/macro-assembler-x64.cc @@ -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()); } }