X64: Convert HeapNumbers that contain valid smi values to smis in binop-stub.
authorlrn@chromium.org <lrn@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 8 Apr 2011 12:34:00 +0000 (12:34 +0000)
committerlrn@chromium.org <lrn@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 8 Apr 2011 12:34:00 +0000 (12:34 +0000)
When the TypeRecordingBinaryOpStub expect smi values as input, they might
sometimes come as HeapNumbers. The transition code will detect the heap numbers
as holding values that are valid smi values, and will not change the expectations.
However, the stub didn't handle HeapNumbers and always tried to transition again.

Review URL: http://codereview.chromium.org/6812046

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7560 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/x64/code-stubs-x64.cc
src/x64/macro-assembler-x64.cc
src/x64/macro-assembler-x64.h

index 2098ed12f5de2684889d8187c61624903989a86d..378e8a751c12a1f3dc4c17ab8be372905bcdd986 100644 (file)
@@ -460,6 +460,25 @@ 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 (if one is
+  // provided), or it falls through at the end of the code (if on_success
+  // is NULL).
+  // On success, both first and second holds Smi tagged values.
+  // One of first or second must be non-Smi when entering.
+  static void NumbersToSmis(MacroAssembler* masm,
+                            Register first,
+                            Register second,
+                            Register scratch1,
+                            Register scratch2,
+                            Register scratch3,
+                            Label* on_success,
+                            Label* on_not_smis);
 };
 
 
@@ -1105,29 +1124,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 +1186,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 +1251,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 +2088,62 @@ 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_map = scratch3;
+  Register smi_result = scratch1;
+  Label done;
+
+  __ LoadRoot(heap_number_map, Heap::kHeapNumberMapRootIndex);
+
+  NearLabel first_smi, check_second;
+  __ JumpIfSmi(first, &first_smi);
+  __ cmpq(FieldOperand(first, HeapObject::kMapOffset), heap_number_map);
+  __ 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 != NULL) ? on_success : &done);
+  __ bind(&first_smi);
+  if (FLAG_debug_code) {
+    // Second should be non-smi if we get here.
+    __ AbortIfSmi(second);
+  }
+  __ cmpq(FieldOperand(second, HeapObject::kMapOffset), heap_number_map);
+  __ 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);
+  if (on_success != NULL) {
+    __ jmp(on_success);
+  } else {
+    __ bind(&done);
+  }
+}
+
+
 void GenericUnaryOpStub::Generate(MacroAssembler* masm) {
   Label slow, done;
 
index bf4e22e026a850076d54b601dac981dded40b63a..b0971663a03fc44bae2863826cb49db67c8d9cf7 100644 (file)
@@ -1320,6 +1320,7 @@ void MacroAssembler::SmiAndConstant(Register dst, Register src, Smi* constant) {
 
 void MacroAssembler::SmiOr(Register dst, Register src1, Register src2) {
   if (!dst.is(src1)) {
+    ASSERT(!src1.is(src2));
     movq(dst, src1);
   }
   or_(dst, src2);
@@ -1340,6 +1341,7 @@ void MacroAssembler::SmiOrConstant(Register dst, Register src, Smi* constant) {
 
 void MacroAssembler::SmiXor(Register dst, Register src1, Register src2) {
   if (!dst.is(src1)) {
+    ASSERT(!src1.is(src2));
     movq(dst, src1);
   }
   xor_(dst, src2);
index 8086388b12854251d557cfff29d561be4514912f..4c177205b6be4f143933f751678641b9dbc14f52 100644 (file)
@@ -323,6 +323,16 @@ class MacroAssembler: public Assembler {
                                            Register src,
                                            int power);
 
+  // Perform the logical or of two smi values and return a smi value.
+  // If either argument is not a smi, jump to on_not_smis and retain
+  // the original values of source registers. The destination register
+  // may be changed if it's not one of the source registers.
+  template <typename LabelType>
+  void SmiOrIfSmis(Register dst,
+                   Register src1,
+                   Register src2,
+                   LabelType* on_not_smis);
+
 
   // Simple comparison of smis.  Both sides must be known smis to use these,
   // otherwise use Cmp.
@@ -1789,6 +1799,24 @@ void MacroAssembler::JumpUnlessBothNonNegativeSmi(Register src1,
 }
 
 
+template <typename LabelType>
+void MacroAssembler::SmiOrIfSmis(Register dst, Register src1, Register src2,
+                                 LabelType* on_not_smis) {
+  if (dst.is(src1) || dst.is(src2)) {
+    ASSERT(!src1.is(kScratchRegister));
+    ASSERT(!src2.is(kScratchRegister));
+    movq(kScratchRegister, src1);
+    or_(kScratchRegister, src2);
+    JumpIfNotSmi(kScratchRegister, on_not_smis);
+    movq(dst, kScratchRegister);
+  } else {
+    movq(dst, src1);
+    or_(dst, src2);
+    JumpIfNotSmi(dst, on_not_smis);
+  }
+}
+
+
 template <typename LabelType>
 void MacroAssembler::JumpIfNotString(Register object,
                                      Register object_map,