Revision: 3041 Author: [email protected] Date: Thu Oct 8 07:27:46 2009 Log: Optimize calls to GenericBinaryStub.
The calls to GenericBinaryStub can now pass the arguments in registers instead of on the stack. It is supported for ADD, SUB, MUL and DIV. The convention in GenericBinaryStub is not changed so the left operand is passed in edx and the right one in eax. When the stub contains smi code arguments are always passed on the stack as the smi code has to have left and right operands on eax and ebx, so moving from edx,eax to eax,ebx is not worth it and the smi code also trashes the registers so if arguments where passed in registers they would have to be saved on the stack anyway. Added flags to disable the use of certain Intel CPU features to make it easier to test different code paths. Review URL: http://codereview.chromium.org/246075 http://code.google.com/p/v8/source/detail?r=3041 Modified: /branches/bleeding_edge/src/flag-definitions.h /branches/bleeding_edge/src/ia32/assembler-ia32.h /branches/bleeding_edge/src/ia32/codegen-ia32.cc /branches/bleeding_edge/src/ia32/codegen-ia32.h /branches/bleeding_edge/src/v8-counters.h /branches/bleeding_edge/src/x64/assembler-x64.h ======================================= --- /branches/bleeding_edge/src/flag-definitions.h Fri Oct 2 03:18:14 2009 +++ /branches/bleeding_edge/src/flag-definitions.h Thu Oct 8 07:27:46 2009 @@ -96,7 +96,7 @@ // #define FLAG FLAG_FULL -// assembler-ia32.cc / assembler-arm.cc +// assembler-ia32.cc / assembler-arm.cc / assembler-x64.cc DEFINE_bool(debug_code, false, "generate extra code (comments, assertions) for debugging") DEFINE_bool(emit_branch_hints, false, "emit branch hints") @@ -104,6 +104,16 @@ "eliminate redundant push/pops in assembly code") DEFINE_bool(print_push_pop_elimination, false, "print elimination of redundant push/pops in assembly code") +DEFINE_bool(enable_sse2, true, + "enable use of SSE2 instructions if available") +DEFINE_bool(enable_sse3, true, + "enable use of SSE3 instructions if available") +DEFINE_bool(enable_cmov, true, + "enable use of CMOV instruction if available") +DEFINE_bool(enable_rdtsc, true, + "enable use of RDTSC instruction if available") +DEFINE_bool(enable_sahf, true, + "enable use of SAHF instruction if available (X64 only)") // bootstrapper.cc DEFINE_string(expose_natives_as, NULL, "expose natives in global object") ======================================= --- /branches/bleeding_edge/src/ia32/assembler-ia32.h Thu Oct 8 00:09:46 2009 +++ /branches/bleeding_edge/src/ia32/assembler-ia32.h Thu Oct 8 07:27:46 2009 @@ -367,6 +367,10 @@ static void Probe(); // Check whether a feature is supported by the target CPU. static bool IsSupported(Feature f) { + if (f == SSE2 && !FLAG_enable_sse2) return false; + if (f == SSE3 && !FLAG_enable_sse3) return false; + if (f == CMOV && !FLAG_enable_cmov) return false; + if (f == RDTSC && !FLAG_enable_rdtsc) return false; return (supported_ & (static_cast<uint64_t>(1) << f)) != 0; } // Check whether a feature is currently enabled. ======================================= --- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Wed Sep 30 02:24:46 2009 +++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Thu Oct 8 07:27:46 2009 @@ -824,10 +824,8 @@ void DeferredInlineBinaryOperation::Generate() { - __ push(left_); - __ push(right_); - GenericBinaryOpStub stub(op_, mode_, SMI_CODE_INLINED); - __ CallStub(&stub); + GenericBinaryOpStub stub(op_, mode_, NO_SMI_CODE_IN_STUB); + stub.GenerateCall(masm_, left_, right_); if (!dst_.is(eax)) __ mov(dst_, eax); } @@ -856,16 +854,16 @@ // Bit operations always assume they likely operate on Smis. Still only // generate the inline Smi check code if this operation is part of a loop. flags = (loop_nesting() > 0) - ? SMI_CODE_INLINED - : SMI_CODE_IN_STUB; + ? NO_SMI_CODE_IN_STUB + : NO_GENERIC_BINARY_FLAGS; break; default: // By default only inline the Smi check code for likely smis if this // operation is part of a loop. flags = ((loop_nesting() > 0) && type->IsLikelySmi()) - ? SMI_CODE_INLINED - : SMI_CODE_IN_STUB; + ? NO_SMI_CODE_IN_STUB + : NO_GENERIC_BINARY_FLAGS; break; } @@ -924,16 +922,15 @@ return; } - if (flags == SMI_CODE_INLINED && !generate_no_smi_code) { + if (((flags & NO_SMI_CODE_IN_STUB) != 0) && !generate_no_smi_code) { LikelySmiBinaryOperation(op, &left, &right, overwrite_mode); } else { frame_->Push(&left); frame_->Push(&right); // If we know the arguments aren't smis, use the binary operation stub // that does not check for the fast smi case. - // The same stub is used for NO_SMI_CODE and SMI_CODE_INLINED. if (generate_no_smi_code) { - flags = SMI_CODE_INLINED; + flags = NO_SMI_CODE_IN_STUB; } GenericBinaryOpStub stub(op, overwrite_mode, flags); Result answer = frame_->CallStub(&stub, 2); @@ -1376,14 +1373,12 @@ void DeferredInlineSmiOperation::Generate() { - __ push(src_); - __ push(Immediate(value_)); // For mod we don't generate all the Smi code inline. GenericBinaryOpStub stub( op_, overwrite_mode_, - (op_ == Token::MOD) ? SMI_CODE_IN_STUB : SMI_CODE_INLINED); - __ CallStub(&stub); + (op_ == Token::MOD) ? NO_GENERIC_BINARY_FLAGS : NO_SMI_CODE_IN_STUB); + stub.GenerateCall(masm_, src_, value_); if (!dst_.is(eax)) __ mov(dst_, eax); } @@ -1417,10 +1412,8 @@ void DeferredInlineSmiOperationReversed::Generate() { - __ push(Immediate(value_)); - __ push(src_); - GenericBinaryOpStub igostub(op_, overwrite_mode_, SMI_CODE_INLINED); - __ CallStub(&igostub); + GenericBinaryOpStub igostub(op_, overwrite_mode_, NO_SMI_CODE_IN_STUB); + igostub.GenerateCall(masm_, value_, src_); if (!dst_.is(eax)) __ mov(dst_, eax); } @@ -1449,10 +1442,8 @@ void DeferredInlineSmiAdd::Generate() { // Undo the optimistic add operation and call the shared stub. __ sub(Operand(dst_), Immediate(value_)); - __ push(dst_); - __ push(Immediate(value_)); - GenericBinaryOpStub igostub(Token::ADD, overwrite_mode_, SMI_CODE_INLINED); - __ CallStub(&igostub); + GenericBinaryOpStub igostub(Token::ADD, overwrite_mode_, NO_SMI_CODE_IN_STUB); + igostub.GenerateCall(masm_, dst_, value_); if (!dst_.is(eax)) __ mov(dst_, eax); } @@ -1481,10 +1472,8 @@ void DeferredInlineSmiAddReversed::Generate() { // Undo the optimistic add operation and call the shared stub. __ sub(Operand(dst_), Immediate(value_)); - __ push(Immediate(value_)); - __ push(dst_); - GenericBinaryOpStub igostub(Token::ADD, overwrite_mode_, SMI_CODE_INLINED); - __ CallStub(&igostub); + GenericBinaryOpStub igostub(Token::ADD, overwrite_mode_, NO_SMI_CODE_IN_STUB); + igostub.GenerateCall(masm_, value_, dst_); if (!dst_.is(eax)) __ mov(dst_, eax); } @@ -1514,10 +1503,8 @@ void DeferredInlineSmiSub::Generate() { // Undo the optimistic sub operation and call the shared stub. __ add(Operand(dst_), Immediate(value_)); - __ push(dst_); - __ push(Immediate(value_)); - GenericBinaryOpStub igostub(Token::SUB, overwrite_mode_, SMI_CODE_INLINED); - __ CallStub(&igostub); + GenericBinaryOpStub igostub(Token::SUB, overwrite_mode_, NO_SMI_CODE_IN_STUB); + igostub.GenerateCall(masm_, dst_, value_); if (!dst_.is(eax)) __ mov(dst_, eax); } @@ -6521,6 +6508,116 @@ __ mov(eax, 0); __ ret(1 * kPointerSize); } + + +void GenericBinaryOpStub::GenerateCall( + MacroAssembler* masm, + Register left, + Register right) { + if (!ArgsInRegistersSupported()) { + // Only pass arguments in registers if there is no smi code in the stub. + __ push(left); + __ push(right); + } else { + // The calling convention with registers is left in edx and right in eax. + __ IncrementCounter(&Counters::generic_binary_stub_calls_regs, 1); + if (!(left.is(edx) && right.is(eax))) { + if (left.is(eax) && right.is(edx)) { + if (IsOperationCommutative()) { + SetArgsReversed(); + } else { + __ xchg(left, right); + } + } else if (left.is(edx)) { + __ mov(eax, right); + } else if (left.is(eax)) { + if (IsOperationCommutative()) { + __ mov(edx, right); + SetArgsReversed(); + } else { + __ mov(edx, left); + __ mov(eax, right); + } + } else if (right.is(edx)) { + if (IsOperationCommutative()) { + __ mov(eax, left); + SetArgsReversed(); + } else { + __ mov(eax, right); + __ mov(edx, left); + } + } else if (right.is(eax)) { + __ mov(edx, left); + } else { + __ mov(edx, left); + __ mov(eax, right); + } + } + + // Update flags to indicate that arguments are in registers. + SetArgsInRegisters(); + } + + // Call the stub. + __ CallStub(this); +} + + +void GenericBinaryOpStub::GenerateCall( + MacroAssembler* masm, + Register left, + Smi* right) { + if (!ArgsInRegistersSupported()) { + // Only pass arguments in registers if there is no smi code in the stub. + __ push(left); + __ push(Immediate(right)); + } else { + // Adapt arguments to the calling convention left in edx and right in eax. + if (left.is(edx)) { + __ mov(eax, Immediate(right)); + } else if (left.is(eax) && IsOperationCommutative()) { + __ mov(edx, Immediate(right)); + SetArgsReversed(); + } else { + __ mov(edx, left); + __ mov(eax, Immediate(right)); + } + + // Update flags to indicate that arguments are in registers. + SetArgsInRegisters(); + } + + // Call the stub. + __ CallStub(this); +} + + +void GenericBinaryOpStub::GenerateCall( + MacroAssembler* masm, + Smi* left, + Register right) { + if (flags_ != NO_SMI_CODE_IN_STUB) { + // Only pass arguments in registers if there is no smi code in the stub. + __ push(Immediate(left)); + __ push(right); + } else { + // Adapt arguments to the calling convention left in edx and right in eax. + bool is_commutative = (op_ == (Token::ADD) || (op_ == Token::MUL)); + if (right.is(eax)) { + __ mov(edx, Immediate(left)); + } else if (right.is(edx) && is_commutative) { + __ mov(eax, Immediate(left)); + } else { + __ mov(edx, Immediate(left)); + __ mov(eax, right); + } + // Update flags to indicate that arguments are in registers. + SetArgsInRegisters(); + } + + // Call the stub. + __ CallStub(this); +} void GenericBinaryOpStub::GenerateSmiCode(MacroAssembler* masm, Label* slow) { @@ -6670,22 +6767,23 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { Label call_runtime; - if (flags_ == SMI_CODE_IN_STUB) { - // The fast case smi code wasn't inlined in the stub caller - // code. Generate it here to speed up common operations. + __ IncrementCounter(&Counters::generic_binary_stub_calls, 1); + + // Generate fast case smi code if requested. This flag is set when the fast + // case smi code is not generated by the caller. Generating it here will speed + // up common operations. + if (HasSmiCodeInStub()) { Label slow; - __ mov(ebx, Operand(esp, 1 * kPointerSize)); // get y - __ mov(eax, Operand(esp, 2 * kPointerSize)); // get x + __ mov(ebx, Operand(esp, 1 * kPointerSize)); + __ mov(eax, Operand(esp, 2 * kPointerSize)); GenerateSmiCode(masm, &slow); - __ ret(2 * kPointerSize); // remove both operands - + GenerateReturn(masm); // Too bad. The fast case smi code didn't succeed. __ bind(&slow); } - // Setup registers. - __ mov(eax, Operand(esp, 1 * kPointerSize)); // get y - __ mov(edx, Operand(esp, 2 * kPointerSize)); // get x + // Make sure the arguments are in edx and eax. + GenerateLoadArguments(masm); // Floating point case. switch (op_) { @@ -6719,19 +6817,24 @@ __ test(eax, Immediate(kSmiTagMask)); __ j(not_zero, &skip_allocation, not_taken); // Fall through! - case NO_OVERWRITE: + case NO_OVERWRITE: { + // Allocate a heap number for the result. Keep eax and edx intact + // for the possible runtime call. FloatingPointHelper::AllocateHeapNumber(masm, &call_runtime, ecx, - edx, - eax); + no_reg, + ebx); + // Now eax can be overwritten losing one of the arguments as we are + // now done and will not need it any more. + __ mov(eax, ebx); __ bind(&skip_allocation); break; + } default: UNREACHABLE(); } __ movdbl(FieldOperand(eax, HeapNumber::kValueOffset), xmm0); - __ ret(2 * kPointerSize); - + GenerateReturn(masm); } else { // SSE2 not available, use FPU. FloatingPointHelper::CheckFloatOperands(masm, &call_runtime, ebx); // Allocate a heap number, if needed. @@ -6747,11 +6850,16 @@ __ j(not_zero, &skip_allocation, not_taken); // Fall through! case NO_OVERWRITE: + // Allocate a heap number for the result. Keep eax and edx intact + // for the possible runtime call. FloatingPointHelper::AllocateHeapNumber(masm, &call_runtime, ecx, - edx, - eax); + no_reg, + ebx); + // Now eax can be overwritten losing one of the arguments as we are + // now done and will not need it any more. + __ mov(eax, ebx); __ bind(&skip_allocation); break; default: UNREACHABLE(); @@ -6766,7 +6874,7 @@ default: UNREACHABLE(); } __ fstp_d(FieldOperand(eax, HeapNumber::kValueOffset)); - __ ret(2 * kPointerSize); + GenerateReturn(masm); } } case Token::MOD: { @@ -6899,8 +7007,20 @@ } // If all else fails, use the runtime system to get the correct - // result. + // result. If arguments was passed in registers now place them on the + // stack in the correct order. __ bind(&call_runtime); + if (HasArgumentsInRegisters()) { + __ pop(ecx); + if (HasArgumentsReversed()) { + __ push(eax); + __ push(edx); + } else { + __ push(edx); + __ push(eax); + } + __ push(ecx); + } switch (op_) { case Token::ADD: { // Test for string arguments before calling runtime. @@ -6975,6 +7095,26 @@ UNREACHABLE(); } } + + +void GenericBinaryOpStub::GenerateLoadArguments(MacroAssembler* masm) { + // If arguments are not passed in registers read them from the stack. + if (!HasArgumentsInRegisters()) { + __ mov(eax, Operand(esp, 1 * kPointerSize)); + __ mov(edx, Operand(esp, 2 * kPointerSize)); + } +} + + +void GenericBinaryOpStub::GenerateReturn(MacroAssembler* masm) { + // If arguments are not passed in registers remove them from the stack before + // returning. + if (!HasArgumentsInRegisters()) { + __ ret(2 * kPointerSize); // Remove both operands + } else { + __ ret(0); + } +} void FloatingPointHelper::AllocateHeapNumber(MacroAssembler* masm, ======================================= --- /branches/bleeding_edge/src/ia32/codegen-ia32.h Mon Sep 28 05:01:05 2009 +++ /branches/bleeding_edge/src/ia32/codegen-ia32.h Thu Oct 8 07:27:46 2009 @@ -604,47 +604,62 @@ }; -// Flag that indicates whether or not the code that handles smi arguments -// should be placed in the stub, inlined, or omitted entirely. +// Flag that indicates whether how to generate code for the stub. enum GenericBinaryFlags { - SMI_CODE_IN_STUB, - SMI_CODE_INLINED + NO_GENERIC_BINARY_FLAGS = 0, + NO_SMI_CODE_IN_STUB = 1 << 0 // Omit smi code in stub. }; class GenericBinaryOpStub: public CodeStub { public: - GenericBinaryOpStub(Token::Value op, + GenericBinaryOpStub(Token::Value operation, OverwriteMode mode, GenericBinaryFlags flags) - : op_(op), mode_(mode), flags_(flags) { + : op_(operation), + mode_(mode), + flags_(flags), + args_in_registers_(false), + args_reversed_(false) { use_sse3_ = CpuFeatures::IsSupported(CpuFeatures::SSE3); ASSERT(OpBits::is_valid(Token::NUM_TOKENS)); } - void GenerateSmiCode(MacroAssembler* masm, Label* slow); + // Generate code to call the stub with the supplied arguments. This will add + // code at the call site to prepare arguments either in registers or on the + // stack together with the actual call. + void GenerateCall(MacroAssembler* masm, Register left, Register right); + void GenerateCall(MacroAssembler* masm, Register left, Smi* right); + void GenerateCall(MacroAssembler* masm, Smi* left, Register right); private: Token::Value op_; OverwriteMode mode_; GenericBinaryFlags flags_; + bool args_in_registers_; // Arguments passed in registers not on the stack. + bool args_reversed_; // Left and right argument are swapped. bool use_sse3_; const char* GetName(); #ifdef DEBUG void Print() { - PrintF("GenericBinaryOpStub (op %s), (mode %d, flags %d)\n", + PrintF("GenericBinaryOpStub (op %s), " + "(mode %d, flags %d, registers %d, reversed %d)\n", Token::String(op_), static_cast<int>(mode_), - static_cast<int>(flags_)); + static_cast<int>(flags_), + static_cast<int>(args_in_registers_), + static_cast<int>(args_reversed_)); } #endif - // Minor key encoding in 16 bits FSOOOOOOOOOOOOMM. + // Minor key encoding in 16 bits FRASOOOOOOOOOOMM. class ModeBits: public BitField<OverwriteMode, 0, 2> {}; - class OpBits: public BitField<Token::Value, 2, 12> {}; - class SSE3Bits: public BitField<bool, 14, 1> {}; + class OpBits: public BitField<Token::Value, 2, 10> {}; + class SSE3Bits: public BitField<bool, 12, 1> {}; + class ArgsInRegistersBits: public BitField<bool, 13, 1> {}; + class ArgsReversedBits: public BitField<bool, 14, 1> {}; class FlagBits: public BitField<GenericBinaryFlags, 15, 1> {}; Major MajorKey() { return GenericBinaryOp; } @@ -653,9 +668,30 @@ return OpBits::encode(op_) | ModeBits::encode(mode_) | FlagBits::encode(flags_) - | SSE3Bits::encode(use_sse3_); - } + | SSE3Bits::encode(use_sse3_) + | ArgsInRegistersBits::encode(args_in_registers_) + | ArgsReversedBits::encode(args_reversed_); + } + void Generate(MacroAssembler* masm); + void GenerateSmiCode(MacroAssembler* masm, Label* slow); + void GenerateLoadArguments(MacroAssembler* masm); + void GenerateReturn(MacroAssembler* masm); + + bool ArgsInRegistersSupported() { + return ((op_ == Token::ADD) || (op_ == Token::SUB) + || (op_ == Token::MUL) || (op_ == Token::DIV)) + && flags_ != NO_SMI_CODE_IN_STUB; + } + bool IsOperationCommutative() { + return (op_ == Token::ADD) || (op_ == Token::MUL); + } + + void SetArgsInRegisters() { args_in_registers_ = true; } + void SetArgsReversed() { args_reversed_ = true; } + bool HasSmiCodeInStub() { return (flags_ & NO_SMI_CODE_IN_STUB) == 0; } + bool HasArgumentsInRegisters() { return args_in_registers_; } + bool HasArgumentsReversed() { return args_reversed_; } }; ======================================= --- /branches/bleeding_edge/src/v8-counters.h Wed Sep 16 04:17:57 2009 +++ /branches/bleeding_edge/src/v8-counters.h Thu Oct 8 07:27:46 2009 @@ -150,7 +150,9 @@ SC(reloc_info_count, V8.RelocInfoCount) \ SC(reloc_info_size, V8.RelocInfoSize) \ SC(zone_segment_bytes, V8.ZoneSegmentBytes) \ - SC(compute_entry_frame, V8.ComputeEntryFrame) + SC(compute_entry_frame, V8.ComputeEntryFrame) \ + SC(generic_binary_stub_calls, V8.GenericBinaryStubCalls) \ + SC(generic_binary_stub_calls_regs, V8.GenericBinaryStubCallsRegs) // This file contains all the v8 counters that are in use. ======================================= --- /branches/bleeding_edge/src/x64/assembler-x64.h Thu Oct 8 05:36:12 2009 +++ /branches/bleeding_edge/src/x64/assembler-x64.h Thu Oct 8 07:27:46 2009 @@ -376,6 +376,11 @@ static void Probe(); // Check whether a feature is supported by the target CPU. static bool IsSupported(Feature f) { + if (f == SSE2 && !FLAG_enable_sse2) return false; + if (f == SSE3 && !FLAG_enable_sse3) return false; + if (f == CMOV && !FLAG_enable_cmov) return false; + if (f == RDTSC && !FLAG_enable_rdtsc) return false; + if (f == SAHF && !FLAG_enable_sahf) return false; return (supported_ & (V8_UINT64_C(1) << f)) != 0; } // Check whether a feature is currently enabled. --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
