From 48f2458bd9db32567a03e2f44b86194d84cc4b7a Mon Sep 17 00:00:00 2001 From: "whesse@chromium.org" Date: Mon, 21 Jun 2010 09:04:44 +0000 Subject: [PATCH] Avoid redundant smi check in x64 loading of floats into SSE2 registers. Review URL: http://codereview.chromium.org/2862004 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4903 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/x64/codegen-x64.cc | 385 +++++++++++++------------------------------------ 1 file changed, 100 insertions(+), 285 deletions(-) diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 4df2c34..8e6b66f 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -262,69 +262,19 @@ class DeferredInlineSmiOperationReversed: public DeferredCode { class FloatingPointHelper : public AllStatic { public: - // Code pattern for loading a floating point value. Input value must - // be either a smi or a heap number object (fp value). Requirements: - // operand on TOS+1. Returns operand as floating point number on FPU - // stack. - static void LoadFloatOperand(MacroAssembler* masm, Register scratch); - - // Code pattern for loading a floating point value. Input value must - // be either a smi or a heap number object (fp value). Requirements: - // operand in src register. Returns operand as floating point number - // in XMM register. May destroy src register. - static void LoadFloatOperand(MacroAssembler* masm, - Register src, - XMMRegister dst); - - // Code pattern for loading a possible number into a XMM register. - // If the contents of src is not a number, control branches to - // the Label not_number. If contents of src is a smi or a heap number - // object (fp value), it is loaded into the XMM register as a double. - // The register src is not changed, and src may not be kScratchRegister. - static void LoadFloatOperand(MacroAssembler* masm, - Register src, - XMMRegister dst, - Label *not_number); - - // Code pattern for loading floating point values. Input values must - // be either smi or heap number objects (fp values). Requirements: - // operand_1 in rdx, operand_2 in rax; Returns operands as - // floating point numbers in XMM registers. - static void LoadFloatOperands(MacroAssembler* masm, - XMMRegister dst1, - XMMRegister dst2); - - // Similar to LoadFloatOperands, assumes that the operands are smis. - static void LoadFloatOperandsFromSmis(MacroAssembler* masm, - XMMRegister dst1, - XMMRegister dst2); - - // Code pattern for loading floating point values onto the fp stack. - // Input values must be either smi or heap number objects (fp values). - // Requirements: - // Register version: operands in registers lhs and rhs. - // Stack version: operands on TOS+1 and TOS+2. - // Returns operands as floating point numbers on fp stack. - static void LoadFloatOperands(MacroAssembler* masm, - Register lhs, - Register rhs); - - // Test if operands are smi or number objects (fp). Requirements: - // operand_1 in rax, operand_2 in rdx; falls through on float or smi - // operands, jumps to the non_float label otherwise. - static void CheckNumberOperands(MacroAssembler* masm, - Label* non_float); - // As CheckNumberOperands above, but expects the HeapNumber map in - // a register. - static void CheckNumberOperands(MacroAssembler* masm, - Label* non_float, - Register heap_number_map); + // Load the operands from rdx and rax into xmm0 and xmm1, as doubles. + // If the operands are not both numbers, jump to not_numbers. + // Leaves rdx and rax unchanged. SmiOperands assumes both are smis. + // NumberOperands assumes both are smis or heap numbers. + static void LoadSSE2SmiOperands(MacroAssembler* masm); + static void LoadSSE2NumberOperands(MacroAssembler* masm); + static void LoadSSE2UnknownOperands(MacroAssembler* masm, + Label* not_numbers); // Takes the operands in rdx and rax and loads them as integers in rax // and rcx. static void LoadAsIntegers(MacroAssembler* masm, - Label* operand_conversion_failure, - Register heap_number_map); + Label* operand_conversion_failure); }; @@ -9112,11 +9062,7 @@ void CompareStub::Generate(MacroAssembler* masm) { if (include_number_compare_) { Label non_number_comparison; Label unordered; - FloatingPointHelper::LoadFloatOperand(masm, rdx, xmm0, - &non_number_comparison); - FloatingPointHelper::LoadFloatOperand(masm, rax, xmm1, - &non_number_comparison); - + FloatingPointHelper::LoadSSE2UnknownOperands(masm, &non_number_comparison); __ ucomisd(xmm0, xmm1); // Don't base result on EFLAGS when a NaN is involved. @@ -9973,86 +9919,72 @@ void StackCheckStub::Generate(MacroAssembler* masm) { } -void FloatingPointHelper::LoadFloatOperand(MacroAssembler* masm, - Register number) { - Label load_smi, done; - - __ JumpIfSmi(number, &load_smi); - __ fld_d(FieldOperand(number, HeapNumber::kValueOffset)); - __ jmp(&done); - - __ bind(&load_smi); - __ SmiToInteger32(number, number); - __ push(number); - __ fild_s(Operand(rsp, 0)); - __ pop(number); - - __ bind(&done); +void FloatingPointHelper::LoadSSE2SmiOperands(MacroAssembler* masm) { + __ SmiToInteger32(kScratchRegister, rdx); + __ cvtlsi2sd(xmm0, kScratchRegister); + __ SmiToInteger32(kScratchRegister, rax); + __ cvtlsi2sd(xmm1, kScratchRegister); } -void FloatingPointHelper::LoadFloatOperand(MacroAssembler* masm, - Register src, - XMMRegister dst) { - ASSERT(!src.is(kScratchRegister)); - Label load_smi, done; - - __ JumpIfSmi(src, &load_smi); - __ movsd(dst, FieldOperand(src, HeapNumber::kValueOffset)); +void FloatingPointHelper::LoadSSE2NumberOperands(MacroAssembler* masm) { + Label load_smi_rdx, load_nonsmi_rax, load_smi_rax, done; + // Load operand in rdx into xmm0. + __ JumpIfSmi(rdx, &load_smi_rdx); + __ movsd(xmm0, FieldOperand(rdx, HeapNumber::kValueOffset)); + // Load operand in rax into xmm1. + __ JumpIfSmi(rax, &load_smi_rax); + __ bind(&load_nonsmi_rax); + __ movsd(xmm1, FieldOperand(rax, HeapNumber::kValueOffset)); __ jmp(&done); - __ bind(&load_smi); - __ SmiToInteger32(kScratchRegister, src); - __ cvtlsi2sd(dst, kScratchRegister); - - __ bind(&done); -} - - -void FloatingPointHelper::LoadFloatOperand(MacroAssembler* masm, - Register src, - XMMRegister dst, - Label* not_number) { - Label load_smi, done; - ASSERT(!src.is(kScratchRegister)); - __ JumpIfSmi(src, &load_smi); - __ LoadRoot(kScratchRegister, Heap::kHeapNumberMapRootIndex); - __ cmpq(FieldOperand(src, HeapObject::kMapOffset), kScratchRegister); - __ j(not_equal, not_number); - __ movsd(dst, FieldOperand(src, HeapNumber::kValueOffset)); - __ jmp(&done); + __ bind(&load_smi_rdx); + __ SmiToInteger32(kScratchRegister, rdx); + __ cvtlsi2sd(xmm0, kScratchRegister); + __ JumpIfNotSmi(rax, &load_nonsmi_rax); - __ bind(&load_smi); - __ SmiToInteger32(kScratchRegister, src); - __ cvtlsi2sd(dst, kScratchRegister); + __ bind(&load_smi_rax); + __ SmiToInteger32(kScratchRegister, rax); + __ cvtlsi2sd(xmm1, kScratchRegister); __ bind(&done); } -void FloatingPointHelper::LoadFloatOperands(MacroAssembler* masm, - XMMRegister dst1, - XMMRegister dst2) { - LoadFloatOperand(masm, rdx, dst1); - LoadFloatOperand(masm, rax, dst2); -} +void FloatingPointHelper::LoadSSE2UnknownOperands(MacroAssembler* masm, + Label* not_numbers) { + Label load_smi_rdx, load_nonsmi_rax, load_smi_rax, load_float_rax, done; + // Load operand in rdx into xmm0, or branch to not_numbers. + __ LoadRoot(rcx, Heap::kHeapNumberMapRootIndex); + __ JumpIfSmi(rdx, &load_smi_rdx); + __ cmpq(FieldOperand(rdx, HeapObject::kMapOffset), rcx); + __ j(not_equal, not_numbers); // Argument in rdx is not a number. + __ movsd(xmm0, FieldOperand(rdx, HeapNumber::kValueOffset)); + // Load operand in rax into xmm1, or branch to not_numbers. + __ JumpIfSmi(rax, &load_smi_rax); + __ bind(&load_nonsmi_rax); + __ cmpq(FieldOperand(rax, HeapObject::kMapOffset), rcx); + __ j(not_equal, not_numbers); + __ movsd(xmm1, FieldOperand(rax, HeapNumber::kValueOffset)); + __ jmp(&done); -void FloatingPointHelper::LoadFloatOperandsFromSmis(MacroAssembler* masm, - XMMRegister dst1, - XMMRegister dst2) { + __ bind(&load_smi_rdx); __ SmiToInteger32(kScratchRegister, rdx); - __ cvtlsi2sd(dst1, kScratchRegister); + __ cvtlsi2sd(xmm0, kScratchRegister); + __ JumpIfNotSmi(rax, &load_nonsmi_rax); + + __ bind(&load_smi_rax); __ SmiToInteger32(kScratchRegister, rax); - __ cvtlsi2sd(dst2, kScratchRegister); + __ cvtlsi2sd(xmm1, kScratchRegister); + __ bind(&done); } // Input: rdx, rax are the left and right objects of a bit op. // Output: rax, rcx are left and right integers for a bit op. void FloatingPointHelper::LoadAsIntegers(MacroAssembler* masm, - Label* conversion_failure, - Register heap_number_map) { + Label* conversion_failure) { // Check float operands. Label arg1_is_object, check_undefined_arg1; Label arg2_is_object, check_undefined_arg2; @@ -10070,7 +10002,8 @@ void FloatingPointHelper::LoadAsIntegers(MacroAssembler* masm, __ jmp(&load_arg2); __ bind(&arg1_is_object); - __ cmpq(FieldOperand(rdx, HeapObject::kMapOffset), heap_number_map); + __ movq(rbx, FieldOperand(rdx, HeapObject::kMapOffset)); + __ CompareRoot(rbx, Heap::kHeapNumberMapRootIndex); __ j(not_equal, &check_undefined_arg1); // Get the untagged integer version of the edx heap number in rcx. IntegerConvert(masm, rdx, rdx); @@ -10091,7 +10024,8 @@ void FloatingPointHelper::LoadAsIntegers(MacroAssembler* masm, __ jmp(&done); __ bind(&arg2_is_object); - __ cmpq(FieldOperand(rax, HeapObject::kMapOffset), heap_number_map); + __ movq(rbx, FieldOperand(rax, HeapObject::kMapOffset)); + __ CompareRoot(rbx, Heap::kHeapNumberMapRootIndex); __ j(not_equal, &check_undefined_arg2); // Get the untagged integer version of the eax heap number in ecx. IntegerConvert(masm, rcx, rax); @@ -10100,74 +10034,6 @@ void FloatingPointHelper::LoadAsIntegers(MacroAssembler* masm, } -void FloatingPointHelper::LoadFloatOperands(MacroAssembler* masm, - Register lhs, - Register rhs) { - Label load_smi_lhs, load_smi_rhs, done_load_lhs, done; - __ JumpIfSmi(lhs, &load_smi_lhs); - __ fld_d(FieldOperand(lhs, HeapNumber::kValueOffset)); - __ bind(&done_load_lhs); - - __ JumpIfSmi(rhs, &load_smi_rhs); - __ fld_d(FieldOperand(rhs, HeapNumber::kValueOffset)); - __ jmp(&done); - - __ bind(&load_smi_lhs); - __ SmiToInteger64(kScratchRegister, lhs); - __ push(kScratchRegister); - __ fild_d(Operand(rsp, 0)); - __ pop(kScratchRegister); - __ jmp(&done_load_lhs); - - __ bind(&load_smi_rhs); - __ SmiToInteger64(kScratchRegister, rhs); - __ push(kScratchRegister); - __ fild_d(Operand(rsp, 0)); - __ pop(kScratchRegister); - - __ bind(&done); -} - - -void FloatingPointHelper::CheckNumberOperands(MacroAssembler* masm, - Label* non_float) { - Label test_other, done; - // Test if both operands are numbers (heap_numbers or smis). - // If not, jump to label non_float. - __ JumpIfSmi(rdx, &test_other); // argument in rdx is OK - __ Cmp(FieldOperand(rdx, HeapObject::kMapOffset), Factory::heap_number_map()); - __ j(not_equal, non_float); // The argument in rdx is not a number. - - __ bind(&test_other); - __ JumpIfSmi(rax, &done); // argument in rax is OK - __ Cmp(FieldOperand(rax, HeapObject::kMapOffset), Factory::heap_number_map()); - __ j(not_equal, non_float); // The argument in rax is not a number. - - // Fall-through: Both operands are numbers. - __ bind(&done); -} - - -void FloatingPointHelper::CheckNumberOperands(MacroAssembler* masm, - Label* non_float, - Register heap_number_map) { - Label test_other, done; - // Test if both operands are numbers (heap_numbers or smis). - // If not, jump to label non_float. - __ JumpIfSmi(rdx, &test_other); // argument in rdx is OK - __ cmpq(FieldOperand(rdx, HeapObject::kMapOffset), heap_number_map); - __ j(not_equal, non_float); // The argument in rdx is not a number. - - __ bind(&test_other); - __ JumpIfSmi(rax, &done); // argument in rax is OK - __ cmpq(FieldOperand(rax, HeapObject::kMapOffset), heap_number_map); - __ j(not_equal, non_float); // The argument in rax is not a number. - - // Fall-through: Both operands are numbers. - __ bind(&done); -} - - const char* GenericBinaryOpStub::GetName() { if (name_ != NULL) return name_; const int len = 100; @@ -10474,15 +10340,15 @@ void GenericBinaryOpStub::GenerateSmiCode(MacroAssembler* masm, Label* slow) { } // left is rdx, right is rax. __ AllocateHeapNumber(rbx, rcx, slow); - FloatingPointHelper::LoadFloatOperandsFromSmis(masm, xmm4, xmm5); + FloatingPointHelper::LoadSSE2SmiOperands(masm); switch (op_) { - case Token::ADD: __ addsd(xmm4, xmm5); break; - case Token::SUB: __ subsd(xmm4, xmm5); break; - case Token::MUL: __ mulsd(xmm4, xmm5); break; - case Token::DIV: __ divsd(xmm4, xmm5); break; + case Token::ADD: __ addsd(xmm0, xmm1); break; + case Token::SUB: __ subsd(xmm0, xmm1); break; + case Token::MUL: __ mulsd(xmm0, xmm1); break; + case Token::DIV: __ divsd(xmm0, xmm1); break; default: UNREACHABLE(); } - __ movsd(FieldOperand(rbx, HeapNumber::kValueOffset), xmm4); + __ movsd(FieldOperand(rbx, HeapNumber::kValueOffset), xmm0); __ movq(rax, rbx); GenerateReturn(masm); } @@ -10527,8 +10393,6 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { } // Floating point case. if (ShouldGenerateFPCode()) { - // Load the HeapNumber map here and use it throughout the FP code. - Register heap_number_map = r9; switch (op_) { case Token::ADD: case Token::SUB: @@ -10543,34 +10407,27 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { // forever for all other operations (also if smi code is skipped). GenerateTypeTransition(masm); } - __ LoadRoot(heap_number_map, Heap::kHeapNumberMapRootIndex); Label not_floats; // rax: y // rdx: x - if (static_operands_type_.IsNumber() && FLAG_debug_code) { - // Assert at runtime that inputs are only numbers. - __ AbortIfNotNumber(rdx); - __ AbortIfNotNumber(rax); - } else { + ASSERT(!static_operands_type_.IsSmi()); + if (static_operands_type_.IsNumber()) { if (FLAG_debug_code) { - __ AbortIfNotRootValue(heap_number_map, - Heap::kHeapNumberMapRootIndex, - "HeapNumberMap register clobbered."); + // Assert at runtime that inputs are only numbers. + __ AbortIfNotNumber(rdx); + __ AbortIfNotNumber(rax); } - FloatingPointHelper::CheckNumberOperands(masm, - &call_runtime, - heap_number_map); + FloatingPointHelper::LoadSSE2NumberOperands(masm); + } else { + FloatingPointHelper::LoadSSE2UnknownOperands(masm, &call_runtime); } - // Fast-case: Both operands are numbers. - // xmm4 and xmm5 are volatile XMM registers. - FloatingPointHelper::LoadFloatOperands(masm, xmm4, xmm5); switch (op_) { - case Token::ADD: __ addsd(xmm4, xmm5); break; - case Token::SUB: __ subsd(xmm4, xmm5); break; - case Token::MUL: __ mulsd(xmm4, xmm5); break; - case Token::DIV: __ divsd(xmm4, xmm5); break; + case Token::ADD: __ addsd(xmm0, xmm1); break; + case Token::SUB: __ subsd(xmm0, xmm1); break; + case Token::MUL: __ mulsd(xmm0, xmm1); break; + case Token::DIV: __ divsd(xmm0, xmm1); break; default: UNREACHABLE(); } // Allocate a heap number, if needed. @@ -10584,24 +10441,9 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { } } switch (mode) { - // TODO(lrn): Allocate this when we first see that the - // left register is a smi (and load it into xmm4). case OVERWRITE_LEFT: __ JumpIfNotSmi(rdx, &skip_allocation); - // Allocate heap number in new space. - __ AllocateInNewSpace(HeapNumber::kSize, - rbx, - rcx, - no_reg, - &call_runtime, - TAG_OBJECT); - if (FLAG_debug_code) { - __ AbortIfNotRootValue(heap_number_map, - Heap::kHeapNumberMapRootIndex, - "HeapNumberMap register clobbered."); - } - __ movq(FieldOperand(rbx, HeapObject::kMapOffset), - heap_number_map); + __ AllocateHeapNumber(rbx, rcx, &call_runtime); __ movq(rdx, rbx); __ bind(&skip_allocation); __ movq(rax, rdx); @@ -10609,32 +10451,18 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { case OVERWRITE_RIGHT: // If the argument in rax is already an object, we skip the // allocation of a heap number. - // TODO(lrn): Allocate the heap number when we first see that the - // right register is a smi (and load it into xmm5). __ JumpIfNotSmi(rax, &skip_allocation); // Fall through! case NO_OVERWRITE: // Allocate a heap number for the result. Keep rax and rdx intact // for the possible runtime call. - __ AllocateInNewSpace(HeapNumber::kSize, - rbx, - rcx, - no_reg, - &call_runtime, - TAG_OBJECT); - if (FLAG_debug_code) { - __ AbortIfNotRootValue(heap_number_map, - Heap::kHeapNumberMapRootIndex, - "HeapNumberMap register clobbered."); - } - __ movq(FieldOperand(rbx, HeapObject::kMapOffset), - heap_number_map); + __ AllocateHeapNumber(rbx, rcx, &call_runtime); __ movq(rax, rbx); __ bind(&skip_allocation); break; default: UNREACHABLE(); } - __ movsd(FieldOperand(rax, HeapNumber::kValueOffset), xmm4); + __ movsd(FieldOperand(rax, HeapNumber::kValueOffset), xmm0); GenerateReturn(masm); __ bind(¬_floats); if (runtime_operands_type_ == BinaryOpIC::DEFAULT && @@ -10659,11 +10487,8 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { case Token::SAR: case Token::SHL: case Token::SHR: { - Label skip_allocation, non_smi_shr_result; - __ LoadRoot(heap_number_map, Heap::kHeapNumberMapRootIndex); - FloatingPointHelper::LoadAsIntegers(masm, - &call_runtime, - heap_number_map); + Label skip_allocation, non_smi_result; + FloatingPointHelper::LoadAsIntegers(masm, &call_runtime); switch (op_) { case Token::BIT_OR: __ orl(rax, rcx); break; case Token::BIT_AND: __ andl(rax, rcx); break; @@ -10675,22 +10500,21 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { } if (op_ == Token::SHR) { // Check if result is negative. This can only happen for a shift - // by zero. + // by zero, which also doesn't update the sign flag. __ testl(rax, rax); - __ j(negative, &non_smi_shr_result); + __ j(negative, &non_smi_result); } - STATIC_ASSERT(kSmiValueSize == 32); - // Tag smi result and return. + __ JumpIfNotValidSmiValue(rax, &non_smi_result); + // Tag smi result, if possible, and return. __ Integer32ToSmi(rax, rax); GenerateReturn(masm); - // All bit-ops except SHR return a signed int32 that can be - // returned immediately as a smi. - if (op_ == Token::SHR) { - ASSERT(non_smi_shr_result.is_linked()); - __ bind(&non_smi_shr_result); + // All ops except SHR return a signed int32 that we load in + // a HeapNumber. + if (op_ != Token::SHR && non_smi_result.is_linked()) { + __ bind(&non_smi_result); // Allocate a heap number if needed. - __ movl(rbx, rax); // rbx holds result value (uint32 value as int64). + __ movsxlq(rbx, rax); // rbx: sign extended 32-bit result switch (mode_) { case OVERWRITE_LEFT: case OVERWRITE_RIGHT: @@ -10701,31 +10525,22 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { __ JumpIfNotSmi(rax, &skip_allocation); // Fall through! case NO_OVERWRITE: - // Allocate heap number in new space. - __ AllocateInNewSpace(HeapNumber::kSize, - rax, - rcx, - no_reg, - &call_runtime, - TAG_OBJECT); - // Set the map. - if (FLAG_debug_code) { - __ AbortIfNotRootValue(heap_number_map, - Heap::kHeapNumberMapRootIndex, - "HeapNumberMap register clobbered."); - } - __ movq(FieldOperand(rax, HeapObject::kMapOffset), - heap_number_map); + __ AllocateHeapNumber(rax, rcx, &call_runtime); __ bind(&skip_allocation); break; default: UNREACHABLE(); } // Store the result in the HeapNumber and return. - __ cvtqsi2sd(xmm0, rbx); - __ movsd(FieldOperand(rax, HeapNumber::kValueOffset), xmm0); + __ movq(Operand(rsp, 1 * kPointerSize), rbx); + __ fild_s(Operand(rsp, 1 * kPointerSize)); + __ fstp_d(FieldOperand(rax, HeapNumber::kValueOffset)); GenerateReturn(masm); } + // SHR should return uint32 - go to runtime for non-smi/negative result. + if (op_ == Token::SHR) { + __ bind(&non_smi_result); + } break; } default: UNREACHABLE(); break; @@ -10758,7 +10573,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) { Label not_strings, both_strings, not_string1, string1, string1_smi2; // If this stub has already generated FP-specific code then the arguments - // are already in rdx, rax. + // are already in rdx, rax if (!ShouldGenerateFPCode() && !HasArgsInRegisters()) { GenerateLoadArguments(masm); } -- 2.7.4