From 26abfc5099a89847320975f3f1d76a8bff9bc056 Mon Sep 17 00:00:00 2001 From: "zhengxing.li" Date: Tue, 8 Mar 2016 08:13:50 -0800 Subject: [PATCH] X87: [turbofan] Further fixing ES6 tail call elimination in Turbofan. port 2aae579cf04b24f605d1ae6b975d67d8dbbee672 (r34566) original commit message: In case when F tail calls G we should also remove the potential arguments adaptor frame for F. This CL introduces two new machine instructions ArchTailCallCodeObjectFromJSFunction and ArchTailCallJSFunctionFromJSFunction which (unlike existing ArchTailCallCodeObject and ArchTailCallJSFunction) also drop arguments adaptor frame if it exists right before jumping to the target function. BUG= Review URL: https://codereview.chromium.org/1777563002 Cr-Commit-Position: refs/heads/master@{#34593} --- src/compiler/x87/code-generator-x87.cc | 52 +++++++++++++++++++- src/compiler/x87/instruction-selector-x87.cc | 1 + src/crankshaft/x87/lithium-codegen-x87.cc | 2 +- src/x87/builtins-x87.cc | 2 +- src/x87/macro-assembler-x87.cc | 30 +++++++---- src/x87/macro-assembler-x87.h | 8 ++- 6 files changed, 79 insertions(+), 16 deletions(-) diff --git a/src/compiler/x87/code-generator-x87.cc b/src/compiler/x87/code-generator-x87.cc index 98b6c88e13..82a2d14c51 100644 --- a/src/compiler/x87/code-generator-x87.cc +++ b/src/compiler/x87/code-generator-x87.cc @@ -362,12 +362,50 @@ void CodeGenerator::AssemblePrepareTailCall(int stack_param_delta) { frame_access_state()->SetFrameAccessToSP(); } +void CodeGenerator::AssemblePopArgumentsAdaptorFrame(Register args_reg, + Register, Register, + Register) { + // There are not enough temp registers left on ia32 for a call instruction + // so we pick some scratch registers and save/restore them manually here. + int scratch_count = 3; + Register scratch1 = ebx; + Register scratch2 = ecx; + Register scratch3 = edx; + DCHECK(!AreAliased(args_reg, scratch1, scratch2, scratch3)); + Label done; + + // Check if current frame is an arguments adaptor frame. + __ cmp(Operand(ebp, StandardFrameConstants::kContextOffset), + Immediate(Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR))); + __ j(not_equal, &done, Label::kNear); + + __ push(scratch1); + __ push(scratch2); + __ push(scratch3); + + // Load arguments count from current arguments adaptor frame (note, it + // does not include receiver). + Register caller_args_count_reg = scratch1; + __ mov(caller_args_count_reg, + Operand(ebp, ArgumentsAdaptorFrameConstants::kLengthOffset)); + __ SmiUntag(caller_args_count_reg); + + ParameterCount callee_args_count(args_reg); + __ PrepareForTailCall(callee_args_count, caller_args_count_reg, scratch2, + scratch3, ReturnAddressState::kOnStack, scratch_count); + __ pop(scratch3); + __ pop(scratch2); + __ pop(scratch1); + + __ bind(&done); +} // Assembles an instruction after register allocation, producing machine code. void CodeGenerator::AssembleArchInstruction(Instruction* instr) { X87OperandConverter i(this, instr); - - switch (ArchOpcodeField::decode(instr->opcode())) { + InstructionCode opcode = instr->opcode(); + ArchOpcode arch_opcode = ArchOpcodeField::decode(opcode); + switch (arch_opcode) { case kArchCallCodeObject: { if (FLAG_debug_code && FLAG_enable_slow_asserts) { __ VerifyX87StackDepth(1); @@ -399,6 +437,7 @@ void CodeGenerator::AssembleArchInstruction(Instruction* instr) { frame_access_state()->ClearSPDelta(); break; } + case kArchTailCallCodeObjectFromJSFunction: case kArchTailCallCodeObject: { if (FLAG_debug_code && FLAG_enable_slow_asserts) { __ VerifyX87StackDepth(1); @@ -406,6 +445,10 @@ void CodeGenerator::AssembleArchInstruction(Instruction* instr) { __ fstp(0); int stack_param_delta = i.InputInt32(instr->InputCount() - 1); AssembleDeconstructActivationRecord(stack_param_delta); + if (arch_opcode == kArchTailCallCodeObjectFromJSFunction) { + AssemblePopArgumentsAdaptorFrame(kJavaScriptCallArgCountRegister, + no_reg, no_reg, no_reg); + } if (HasImmediateInput(instr, 0)) { Handle code = Handle::cast(i.InputHeapObject(0)); __ jmp(code, RelocInfo::CODE_TARGET); @@ -447,6 +490,7 @@ void CodeGenerator::AssembleArchInstruction(Instruction* instr) { frame_access_state()->ClearSPDelta(); break; } + case kArchTailCallJSFunctionFromJSFunction: case kArchTailCallJSFunction: { Register func = i.InputRegister(0); if (FLAG_debug_code) { @@ -460,6 +504,10 @@ void CodeGenerator::AssembleArchInstruction(Instruction* instr) { __ fstp(0); int stack_param_delta = i.InputInt32(instr->InputCount() - 1); AssembleDeconstructActivationRecord(stack_param_delta); + if (arch_opcode == kArchTailCallJSFunctionFromJSFunction) { + AssemblePopArgumentsAdaptorFrame(kJavaScriptCallArgCountRegister, + no_reg, no_reg, no_reg); + } __ jmp(FieldOperand(func, JSFunction::kCodeEntryOffset)); frame_access_state()->ClearSPDelta(); break; diff --git a/src/compiler/x87/instruction-selector-x87.cc b/src/compiler/x87/instruction-selector-x87.cc index f1064d7402..0f2ac1fbbf 100644 --- a/src/compiler/x87/instruction-selector-x87.cc +++ b/src/compiler/x87/instruction-selector-x87.cc @@ -1017,6 +1017,7 @@ void InstructionSelector::EmitPrepareArguments( bool InstructionSelector::IsTailCallAddressImmediate() { return true; } +int InstructionSelector::GetTempsCountForTailCallFromJSFunction() { return 0; } namespace { diff --git a/src/crankshaft/x87/lithium-codegen-x87.cc b/src/crankshaft/x87/lithium-codegen-x87.cc index b79ae64c24..59b90d12eb 100644 --- a/src/crankshaft/x87/lithium-codegen-x87.cc +++ b/src/crankshaft/x87/lithium-codegen-x87.cc @@ -3831,7 +3831,7 @@ void LCodeGen::PrepareForTailCall(const ParameterCount& actual, __ bind(&formal_parameter_count_loaded); __ PrepareForTailCall(actual, caller_args_count_reg, scratch2, scratch3, - ReturnAddressState::kNotOnStack); + ReturnAddressState::kNotOnStack, 0); Comment(";;; }"); } diff --git a/src/x87/builtins-x87.cc b/src/x87/builtins-x87.cc index 161fef6f1e..2139cff6a5 100644 --- a/src/x87/builtins-x87.cc +++ b/src/x87/builtins-x87.cc @@ -1935,7 +1935,7 @@ void PrepareForTailCall(MacroAssembler* masm, Register args_reg, ParameterCount callee_args_count(args_reg); __ PrepareForTailCall(callee_args_count, caller_args_count_reg, scratch2, - scratch3, ReturnAddressState::kOnStack); + scratch3, ReturnAddressState::kOnStack, 0); __ bind(&done); } } // namespace diff --git a/src/x87/macro-assembler-x87.cc b/src/x87/macro-assembler-x87.cc index 3e67318d86..d350343b93 100644 --- a/src/x87/macro-assembler-x87.cc +++ b/src/x87/macro-assembler-x87.cc @@ -2045,10 +2045,10 @@ void MacroAssembler::JumpToExternalReference(const ExternalReference& ext) { jmp(ces.GetCode(), RelocInfo::CODE_TARGET); } -void MacroAssembler::PrepareForTailCall(const ParameterCount& callee_args_count, - Register caller_args_count_reg, - Register scratch0, Register scratch1, - ReturnAddressState ra_state) { +void MacroAssembler::PrepareForTailCall( + const ParameterCount& callee_args_count, Register caller_args_count_reg, + Register scratch0, Register scratch1, ReturnAddressState ra_state, + int number_of_temp_values_after_return_address) { #if DEBUG if (callee_args_count.is_reg()) { DCHECK(!AreAliased(callee_args_count.reg(), caller_args_count_reg, scratch0, @@ -2056,6 +2056,8 @@ void MacroAssembler::PrepareForTailCall(const ParameterCount& callee_args_count, } else { DCHECK(!AreAliased(caller_args_count_reg, scratch0, scratch1)); } + DCHECK(ra_state != ReturnAddressState::kNotOnStack || + number_of_temp_values_after_return_address == 0); #endif // Calculate the destination address where we will put the return address @@ -2063,12 +2065,16 @@ void MacroAssembler::PrepareForTailCall(const ParameterCount& callee_args_count, Register new_sp_reg = scratch0; if (callee_args_count.is_reg()) { sub(caller_args_count_reg, callee_args_count.reg()); - lea(new_sp_reg, Operand(ebp, caller_args_count_reg, times_pointer_size, - StandardFrameConstants::kCallerPCOffset)); + lea(new_sp_reg, + Operand(ebp, caller_args_count_reg, times_pointer_size, + StandardFrameConstants::kCallerPCOffset - + number_of_temp_values_after_return_address * kPointerSize)); } else { lea(new_sp_reg, Operand(ebp, caller_args_count_reg, times_pointer_size, StandardFrameConstants::kCallerPCOffset - - callee_args_count.immediate() * kPointerSize)); + (callee_args_count.immediate() + + number_of_temp_values_after_return_address) * + kPointerSize)); } if (FLAG_debug_code) { @@ -2082,9 +2088,11 @@ void MacroAssembler::PrepareForTailCall(const ParameterCount& callee_args_count, Register tmp_reg = scratch1; if (ra_state == ReturnAddressState::kOnStack) { mov(tmp_reg, Operand(ebp, StandardFrameConstants::kCallerPCOffset)); - mov(Operand(esp, 0), tmp_reg); + mov(Operand(esp, number_of_temp_values_after_return_address * kPointerSize), + tmp_reg); } else { DCHECK(ReturnAddressState::kNotOnStack == ra_state); + DCHECK_EQ(0, number_of_temp_values_after_return_address); Push(Operand(ebp, StandardFrameConstants::kCallerPCOffset)); } @@ -2095,9 +2103,11 @@ void MacroAssembler::PrepareForTailCall(const ParameterCount& callee_args_count, // +2 here is to copy both receiver and return address. Register count_reg = caller_args_count_reg; if (callee_args_count.is_reg()) { - lea(count_reg, Operand(callee_args_count.reg(), 2)); + lea(count_reg, Operand(callee_args_count.reg(), + 2 + number_of_temp_values_after_return_address)); } else { - mov(count_reg, Immediate(callee_args_count.immediate() + 2)); + mov(count_reg, Immediate(callee_args_count.immediate() + 2 + + number_of_temp_values_after_return_address)); // TODO(ishell): Unroll copying loop for small immediate values. } diff --git a/src/x87/macro-assembler-x87.h b/src/x87/macro-assembler-x87.h index 7848a353c1..4feb62dd8f 100644 --- a/src/x87/macro-assembler-x87.h +++ b/src/x87/macro-assembler-x87.h @@ -325,10 +325,14 @@ class MacroAssembler: public Assembler { // |ra_state| defines whether return address is already pushed to stack or // not. Both |callee_args_count| and |caller_args_count_reg| do not include // receiver. |callee_args_count| is not modified, |caller_args_count_reg| - // is trashed. + // is trashed. |number_of_temp_values_after_return_address| specifies + // the number of words pushed to the stack after the return address. This is + // to allow "allocation" of scratch registers that this function requires + // by saving their values on the stack. void PrepareForTailCall(const ParameterCount& callee_args_count, Register caller_args_count_reg, Register scratch0, - Register scratch1, ReturnAddressState ra_state); + Register scratch1, ReturnAddressState ra_state, + int number_of_temp_values_after_return_address); // Invoke the JavaScript function code by either calling or jumping.