X64: Convert HeapNumbers that contain valid smi values to smis in binop-stub.

src/x64/code-stubs-x64.cc

Index: src/x64/code-stubs-x64.cc
diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index 2098ed12f5de2684889d8187c61624903989a86d..8d12512274d3c160d5e4f27f9d960502f3e5ff3d 100644
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -460,6 +460,26 @@ class FloatingPointHelper : public AllStatic {
   // As above, but we know the operands to be numbers. In that case,
   // conversion can't fail.
   static void LoadNumbersAsIntegers(MacroAssembler* masm);
+
+  // Tries to convert two values to smis losslessly.
+  // This fails if either argument is not a Smi nor a HeapNumber,
+  // or if it's a HeapNumber with a value that can't be converted
+  // losslessly to a Smi. In that case, control transitions to the
+  // on_not_smis label.
+  // On success, either control goes to the on_success label,
+  // or it falls through at the end of the code.
+  // (If you just want to fall through in all cases, put the success

[William Hesse, 2011/04/08 12:01:27]

Why let the user of this function make a mistake.  NULL means that all successes
fall through, non-NULL means that all successes go there.

[Lasse Reichstein, 2011/04/08 12:07:46]

Done.

+  // label at the fall-through point).
+  // At this point, both first and second holds Smi tagged values.
+  // One of first or second must be non-Smi.
+  static void NumbersToSmis(MacroAssembler* masm,
+                            Register first,
+                            Register second,
+                            Register scratch1,
+                            Register scratch2,
+                            Register scratch3,
+                            Label* on_success,
+                            Label* on_not_smis);
 };
 
 
@@ -1105,29 +1125,30 @@ void TypeRecordingBinaryOpStub::GenerateSmiCode(MacroAssembler* masm,
     Label* slow,
     SmiCodeGenerateHeapNumberResults allow_heapnumber_results) {
 
+  // Arguments to TypeRecordingBinaryOpStub are in rdx and rax.
+  Register left = rdx;
+  Register right = rax;
+
   // We only generate heapnumber answers for overflowing calculations
-  // for the four basic arithmetic operations.
+  // for the four basic arithmetic operations and logical right shift by 0.
   bool generate_inline_heapnumber_results =
       (allow_heapnumber_results == ALLOW_HEAPNUMBER_RESULTS) &&
       (op_ == Token::ADD || op_ == Token::SUB ||
        op_ == Token::MUL || op_ == Token::DIV || op_ == Token::SHR);
 
-  // Arguments to TypeRecordingBinaryOpStub are in rdx and rax.
-  Register left = rdx;
-  Register right = rax;
-
-
   // Smi check of both operands.  If op is BIT_OR, the check is delayed
   // until after the OR operation.
   Label not_smis;
   Label use_fp_on_smis;
-  Label restore_MOD_registers;  // Only used if op_ == Token::MOD.
+  Label fail;
 
   if (op_ != Token::BIT_OR) {
     Comment smi_check_comment(masm, "-- Smi check arguments");
     __ JumpIfNotBothSmi(left, right, &not_smis);
   }
 
+  Label smi_values;
+  __ bind(&smi_values);
   // Perform the operation.
   Comment perform_smi(masm, "-- Perform smi operation");
   switch (op_) {
@@ -1166,9 +1187,7 @@ void TypeRecordingBinaryOpStub::GenerateSmiCode(MacroAssembler* masm,
 
     case Token::BIT_OR: {
       ASSERT(right.is(rax));
-      __ movq(rcx, right);  // Save the right operand.
-      __ SmiOr(right, right, left);  // BIT_OR is commutative.
-      __ JumpIfNotSmi(right, &not_smis);  // Test delayed until after BIT_OR.
+      __ SmiOrIfSmis(right, right, left, &not_smis);  // BIT_OR is commutative.
       break;
       }
     case Token::BIT_XOR:
@@ -1233,17 +1252,22 @@ void TypeRecordingBinaryOpStub::GenerateSmiCode(MacroAssembler* masm,
       __ movsd(FieldOperand(rcx, HeapNumber::kValueOffset), xmm0);
       __ movq(rax, rcx);
       __ ret(0);
+    } else {
+      __ jmp(&fail);
     }
   }
 
   // 7. Non-smi operands reach the end of the code generated by
   //    GenerateSmiCode, and fall through to subsequent code,
   //    with the operands in rdx and rax.
-  Comment done_comment(masm, "-- Enter non-smi code");
+  //    But first we check if non-smi values are HeapNumbers holding
+  //    values that could be smi.
   __ bind(&not_smis);
-  if (op_ == Token::BIT_OR) {
-    __ movq(right, rcx);
-  }
+  Comment done_comment(masm, "-- Enter non-smi code");
+  FloatingPointHelper::NumbersToSmis(masm, left, right, rbx, rdi, rcx,
+                                     &smi_values, &fail);
+  __ jmp(&smi_values);
+  __ bind(&fail);
 }
 
 
@@ -2065,6 +2089,55 @@ void FloatingPointHelper::LoadSSE2UnknownOperands(MacroAssembler* masm,
 }
 
 
+void FloatingPointHelper::NumbersToSmis(MacroAssembler* masm,
+                                        Register first,
+                                        Register second,
+                                        Register scratch1,
+                                        Register scratch2,
+                                        Register scratch3,
+                                        Label* on_success,
+                                        Label* on_not_smis)   {
+  Register heap_number_root = scratch3;
+  Register smi_result = scratch1;
+  __ LoadRoot(heap_number_root, Heap::kHeapNumberMapRootIndex);

[William Hesse, 2011/04/08 12:01:27]

Shouldn't this be called heap_number_map?

[Lasse Reichstein, 2011/04/08 12:07:46]

Absolutely.

+
+  NearLabel first_smi, check_second;
+  __ JumpIfSmi(first, &first_smi);
+  __ cmpq(FieldOperand(first, HeapObject::kMapOffset), heap_number_root);
+  __ j(not_equal, on_not_smis);
+  // Convert HeapNumber to smi if possible.
+  __ movsd(xmm0, FieldOperand(first, HeapNumber::kValueOffset));
+  __ movq(scratch2, xmm0);
+  __ cvttsd2siq(smi_result, xmm0);
+  // Check if conversion was successful by converting back and
+  // comparing to the original double's bits.
+  __ cvtlsi2sd(xmm1, smi_result);
+  __ movq(kScratchRegister, xmm1);
+  __ cmpq(scratch2, kScratchRegister);
+  __ j(not_equal, on_not_smis);
+  __ Integer32ToSmi(first, smi_result);
+
+  __ bind(&check_second);
+  __ JumpIfSmi(second, on_success);
+  __ bind(&first_smi);
+  if (FLAG_debug_code) {
+    // One of first or second should be non-smi if we get here.

[William Hesse, 2011/04/08 12:01:27]

Second should be non-smi if we get here.

[Lasse Reichstein, 2011/04/08 12:07:46]

Done.

+    __ AbortIfSmi(second);
+  }
+  __ cmpq(FieldOperand(second, HeapObject::kMapOffset), heap_number_root);
+  __ j(not_equal, on_not_smis);
+  // Convert second to smi, if possible.
+  __ movsd(xmm0, FieldOperand(second, HeapNumber::kValueOffset));
+  __ movq(scratch2, xmm0);
+  __ cvttsd2siq(smi_result, xmm0);
+  __ cvtlsi2sd(xmm1, smi_result);
+  __ movq(kScratchRegister, xmm1);
+  __ cmpq(scratch2, kScratchRegister);
+  __ j(not_equal, on_not_smis);
+  __ Integer32ToSmi(second, smi_result);
+}
+
+
 void GenericUnaryOpStub::Generate(MacroAssembler* masm) {
   Label slow, done;