Avoid redundant smi check in x64 loading of floats into SSE2 registers.

src/x64/codegen-x64.cc

Index: src/x64/codegen-x64.cc
===================================================================
--- src/x64/codegen-x64.cc	(revision 4902)
+++ src/x64/codegen-x64.cc	(working copy)
@@ -262,69 +262,19 @@
 
 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);
+  // 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);
 
-  // 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);
-
   // 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 @@
   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 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(&load_smi_rdx);
+  __ SmiToInteger32(kScratchRegister, rdx);
+  __ cvtlsi2sd(xmm0, kScratchRegister);
+  __ JumpIfNotSmi(rax, &load_nonsmi_rax);
 
-  __ bind(&done);
-}
+  __ bind(&load_smi_rax);
+  __ SmiToInteger32(kScratchRegister, rax);
+  __ cvtlsi2sd(xmm1, kScratchRegister);
 
-
-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);
-  __ SmiToInteger32(kScratchRegister, src);
-  __ cvtlsi2sd(dst, 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 @@
   __ 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 @@
   __ 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::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 @@
       }
       // 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 @@
   }
   // 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 @@
           // 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 @@
           }
         }
         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 @@
           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(&not_floats);
         if (runtime_operands_type_ == BinaryOpIC::DEFAULT &&
@@ -10659,11 +10487,8 @@
       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 @@
         }
         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 @@
               __ 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 @@
       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);
       }