Implement truncated d-to-i with a stub on x86

src/ia32/code-stubs-ia32.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 601 matching lines @@
   // from |scratch|, it will contain that int32 value.
   static void CheckSSE2OperandIsInt32(MacroAssembler* masm,
                                       Label* non_int32,
                                       XMMRegister operand,
                                       Register int32_result,
                                       Register scratch,
                                       XMMRegister xmm_scratch);
 };
 
 
-// Get the integer part of a heap number.  Surprisingly, all this bit twiddling
-// is faster than using the built-in instructions on floating point registers.
-// Trashes edi and ebx.  Dest is ecx.  Source cannot be ecx or one of the
-// trashed registers.
-static void IntegerConvert(MacroAssembler* masm,
-                           Register source,
-                           bool use_sse3,
-                           Label* conversion_failure) {
-  ASSERT(!source.is(ecx) && !source.is(edi) && !source.is(ebx));
-  Label done, right_exponent, normal_exponent;
-  Register scratch = ebx;
-  Register scratch2 = edi;
-  // Get exponent word.
-  __ mov(scratch, FieldOperand(source, HeapNumber::kExponentOffset));
-  // Get exponent alone in scratch2.
-  __ mov(scratch2, scratch);
-  __ and_(scratch2, HeapNumber::kExponentMask);
-  __ shr(scratch2, HeapNumber::kExponentShift);
-  __ sub(scratch2, Immediate(HeapNumber::kExponentBias));
-  // Load ecx with zero.  We use this either for the final shift or
-  // for the answer.
-  __ xor_(ecx, ecx);
-  // If the exponent is above 83, the number contains no significant
-  // bits in the range 0..2^31, so the result is zero.
-  static const uint32_t kResultIsZeroExponent = 83;
-  __ cmp(scratch2, Immediate(kResultIsZeroExponent));
+void DoubleToIStub::Generate(MacroAssembler* masm) {
+  Register input_reg = this->source();
+  Register final_result_reg = this->destination();
+  ASSERT(is_truncating());
+
+  Label check_negative, process_64_bits, done, done_no_stash;
+
+  int double_offset = offset();
+
+  // Account for return address and saved regs if input is esp.
+  if (input_reg.is(esp)) double_offset += 3 * kPointerSize;
+
+  MemOperand mantissa_operand(MemOperand(input_reg, double_offset));
+  MemOperand exponent_operand(MemOperand(input_reg,
+                                         double_offset + kPointerSize));
+
+  Register scratch1 = ebx;
+  if (final_result_reg.is(ebx)) {
+    scratch1 = input_reg.is(edx) ? edi : edx;
+  } else if (final_result_reg.is(edx)) {
+    scratch1 = input_reg.is(ebx) ? edi : ebx;
+  } else if (input_reg.is(ebx)) {
+    scratch1 = edx;
+  }

[Yang, 2013/07/11 12:00:27]

Seems easy to introduce bugs in here.

How about something like

  Register scratch1;
  {
    Register scratch_candidates[3] = { ebx, edx, edi };
    for (int i = 0; i < 3; i++) {
      scratch1 = scratch_candidates[i];
      if (!final_result_reg.is(scratch1) && input_reg.is(scratch1)) break;
    }
  }

[danno, 2013/07/11 16:00:23]

Done.

+  Register result_reg = final_result_reg.is(ecx) ? eax : final_result_reg;
+  Register save_reg = final_result_reg.is(ecx) ? eax : ecx;

[Yang, 2013/07/11 12:00:27]

why do we need to spill eax if the result is expected in ecx? Just so that the
esp offset is 3 stack slots?

[danno, 2013/07/11 16:00:23]

No, it's because we use eax above to calculate the result above if the the real
result is ecx, and part of the contract is that we don't clobber registers. I'll
comment.

+  __ push(scratch1);
+  __ push(save_reg);
+
+  bool stash_exponent_copy = !input_reg.is(esp);
+  if (!input_reg.is(ecx)) {
+    __ mov(ecx, exponent_operand);
+    if (stash_exponent_copy) __ push(ecx);
+  }
+  __ mov(scratch1, mantissa_operand);
+  if (CpuFeatures::IsSupported(SSE3)) {
+    CpuFeatureScope scope(masm, SSE3);
+    // Load x87 register with heap number.
+    __ fld_d(mantissa_operand);
+  }
+  if (input_reg.is(ecx)) {
+    __ mov(ecx, exponent_operand);
+    if (stash_exponent_copy) __ push(ecx);
+  }

[Yang, 2013/07/11 12:00:27]

At this point, we have overwritten ecx regardless what input_reg is. Couldn't we
just always load the mantissa onto the FP stack first and then overwrite ecx
with the exponent?

Unless you expect performance difference by loading the exponent earlier, in
which case, ignore this comment.

[danno, 2013/07/11 16:00:23]

Done.

+  __ shr(ecx, HeapNumber::kExponentShift);
+  __ and_(ecx,
+          Immediate(HeapNumber::kExponentMask >> HeapNumber::kExponentShift));

[Yang, 2013/07/11 12:00:27]

Looks like we could first mask and then shift, like we do elsewhere, so that the
mask doesn't have to be shifted. Easier to read imo.

[danno, 2013/07/11 16:00:23]

Done.

+  __ lea(result_reg, MemOperand(ecx, -HeapNumber::kExponentBias));

[Yang, 2013/07/11 12:00:27]

As far as I can see the value in result_reg is not used later on. We could just
compare ecx to (HeapNumber::kExponentBias + HeapNumber::kMantissaBits) directly,
without lea.

Or even use sub instead of cmp, which stores the result in ecx for later and
also sets the flags for the following conditional jump. In line 702-704 we would
then simply neg(ecx).

[danno, 2013/07/11 16:00:23]

This is actually a different test. The cmp and below branch tests a range of
values. You can't do that if you simply do the subtract and check for less that
zero.

+  __ cmp(result_reg, Immediate(HeapNumber::kMantissaBits));
+  __ j(below, &process_64_bits);
+
+  // Result is entirely in lower 32-bits of mantissa
+  int delta = HeapNumber::kExponentBias + Double::kPhysicalSignificandSize;
+  if (CpuFeatures::IsSupported(SSE3)) {
+    __ fstp(0);
+  }
+  __ sub(ecx, Immediate(delta));
+  __ mov(result_reg, Immediate(0));

[Yang, 2013/07/11 12:00:27]

Use Set instead of mov to use xor, which is shorter. Or just use xor inline.

[danno, 2013/07/11 16:00:23]

Done.

+  __ cmp(ecx, Immediate(31));
   __ j(above, &done);
-  if (use_sse3) {
+  __ shl_cl(scratch1);
+  __ jmp(&check_negative);
+
+  __ bind(&process_64_bits);
+  if (CpuFeatures::IsSupported(SSE3)) {
     CpuFeatureScope scope(masm, SSE3);
-    // Check whether the exponent is too big for a 64 bit signed integer.
-    static const uint32_t kTooBigExponent = 63;
-    __ cmp(scratch2, Immediate(kTooBigExponent));
-    __ j(greater_equal, conversion_failure);
-    // Load x87 register with heap number.
-    __ fld_d(FieldOperand(source, HeapNumber::kValueOffset));
-    // Reserve space for 64 bit answer.
-    __ sub(esp, Immediate(sizeof(uint64_t)));  // Nolint.
+    if (stash_exponent_copy) {
+      // Already a copy of the mantissa on the stack, overwrite it.

[Yang, 2013/07/11 12:00:27]

Comment seems wrong. We pushed a copy of the exponent onto the stack, not of the
mantissa.

We also should, at some point, add a STATIC_ASSERT(kDoubleSize == 2 *
kPointerSize), since we do rely on that.

[danno, 2013/07/11 16:00:23]

Done.

+      __ sub(esp, Immediate(kDoubleSize / 2));
+    } else {
+      // Reserve space for 64 bit answer.
+      __ sub(esp, Immediate(kDoubleSize));  // Nolint.
+    }
     // Do conversion, which cannot fail because we checked the exponent.
     __ fisttp_d(Operand(esp, 0));
-    __ mov(ecx, Operand(esp, 0));  // Load low word of answer into ecx.
+    __ mov(result_reg, Operand(esp, 0));  // Load low word of answer as result
     __ add(esp, Immediate(sizeof(uint64_t)));  // Nolint.

[Yang, 2013/07/11 12:00:27]

We should use kDoubleSize for consistency. Also, do we really need Nolint?

[danno, 2013/07/11 16:00:23]

Done.

+    __ jmp(&done_no_stash);
   } else {
-    // Check whether the exponent matches a 32 bit signed int that cannot be
-    // represented by a Smi.  A non-smi 32 bit integer is 1.xxx * 2^30 so the
-    // exponent is 30 (biased).  This is the exponent that we are fastest at and
-    // also the highest exponent we can handle here.
-    const uint32_t non_smi_exponent = 30;
-    __ cmp(scratch2, Immediate(non_smi_exponent));
-    // If we have a match of the int32-but-not-Smi exponent then skip some
-    // logic.
-    __ j(equal, &right_exponent, Label::kNear);
-    // If the exponent is higher than that then go to slow case.  This catches
-    // numbers that don't fit in a signed int32, infinities and NaNs.
-    __ j(less, &normal_exponent, Label::kNear);
+    // Result must be extracted from shifted 32-bit mantissa
+    __ mov(result_reg, Immediate(delta));
+    __ sub(result_reg, ecx);
+    __ mov(ecx, result_reg);

[Yang, 2013/07/11 12:00:27]

sub(ecx, Immediate(delta));
neg(ecx);

should do the same?

Alternatively, the sub has already been done as described above, so we just need
a neg.

[danno, 2013/07/11 16:00:23]

Done.

+    if (stash_exponent_copy) {
+      __ mov(result_reg, MemOperand(esp, 0));
+    } else {
+      __ mov(result_reg, exponent_operand);
+    }
+    __ and_(result_reg,
+            Immediate(static_cast<uint32_t>(Double::kSignificandMask >> 32)));
+    __ add(result_reg,
+           Immediate(static_cast<uint32_t>(Double::kHiddenBit >> 32)));
+    __ shrd(result_reg, scratch1);
+    __ shr_cl(result_reg);
+    __ test(ecx, Immediate(32));
+    if (CpuFeatures::IsSupported(CMOV)) {
+      CpuFeatureScope use_cmov(masm, CMOV);
+      __ cmov(not_equal, scratch1, result_reg);
+    } else {
+      Label skip_mov;
+      __ j(equal, &skip_mov);

[Yang, 2013/07/11 12:00:27]

mark as near jump.

[danno, 2013/07/11 16:00:23]

Done.

+      __ mov(scratch1, result_reg);
+      __ bind(&skip_mov);
+    }
+  }
 
-    {
-      // Handle a big exponent.  The only reason we have this code is that the
-      // >>> operator has a tendency to generate numbers with an exponent of 31.
-      const uint32_t big_non_smi_exponent = 31;
-      __ cmp(scratch2, Immediate(big_non_smi_exponent));
-      __ j(not_equal, conversion_failure);
-      // We have the big exponent, typically from >>>.  This means the number is
-      // in the range 2^31 to 2^32 - 1.  Get the top bits of the mantissa.
-      __ mov(scratch2, scratch);
-      __ and_(scratch2, HeapNumber::kMantissaMask);
-      // Put back the implicit 1.
-      __ or_(scratch2, 1 << HeapNumber::kExponentShift);
-      // Shift up the mantissa bits to take up the space the exponent used to
-      // take. We just orred in the implicit bit so that took care of one and
-      // we want to use the full unsigned range so we subtract 1 bit from the
-      // shift distance.
-      const int big_shift_distance = HeapNumber::kNonMantissaBitsInTopWord - 1;
-      __ shl(scratch2, big_shift_distance);
-      // Get the second half of the double.
-      __ mov(ecx, FieldOperand(source, HeapNumber::kMantissaOffset));
-      // Shift down 21 bits to get the most significant 11 bits or the low
-      // mantissa word.
-      __ shr(ecx, 32 - big_shift_distance);
-      __ or_(ecx, scratch2);
-      // We have the answer in ecx, but we may need to negate it.
-      __ test(scratch, scratch);
-      __ j(positive, &done, Label::kNear);
-      __ neg(ecx);
-      __ jmp(&done, Label::kNear);
-    }
+  // If the double was negative, negate the integer result.
+  __ bind(&check_negative);
+  __ mov(result_reg, scratch1);
+  __ neg(result_reg);
+  if (stash_exponent_copy) {
+    __ cmp(MemOperand(esp, 0), Immediate(0));
+  } else {
+    __ cmp(exponent_operand, Immediate(0));
+  }
+  if (CpuFeatures::IsSupported(CMOV)) {
+    CpuFeatureScope use_cmov(masm, CMOV);
+    __ cmov(greater, result_reg, scratch1);
+  } else {
+    Label skip_mov;
+    __ j(less_equal, &skip_mov);

[Yang, 2013/07/11 12:00:27]

mark as near jump

[danno, 2013/07/11 16:00:23]

Done.

+    __ mov(result_reg, scratch1);
+    __ bind(&skip_mov);
+  }
 
-    __ bind(&normal_exponent);
-    // Exponent word in scratch, exponent in scratch2. Zero in ecx.
-    // We know that 0 <= exponent < 30.
-    __ mov(ecx, Immediate(30));
-    __ sub(ecx, scratch2);
-
-    __ bind(&right_exponent);
-    // Here ecx is the shift, scratch is the exponent word.
-    // Get the top bits of the mantissa.
-    __ and_(scratch, HeapNumber::kMantissaMask);
-    // Put back the implicit 1.
-    __ or_(scratch, 1 << HeapNumber::kExponentShift);
-    // Shift up the mantissa bits to take up the space the exponent used to
-    // take. We have kExponentShift + 1 significant bits int he low end of the
-    // word.  Shift them to the top bits.
-    const int shift_distance = HeapNumber::kNonMantissaBitsInTopWord - 2;
-    __ shl(scratch, shift_distance);
-    // Get the second half of the double. For some exponents we don't
-    // actually need this because the bits get shifted out again, but
-    // it's probably slower to test than just to do it.
-    __ mov(scratch2, FieldOperand(source, HeapNumber::kMantissaOffset));
-    // Shift down 22 bits to get the most significant 10 bits or the low
-    // mantissa word.
-    __ shr(scratch2, 32 - shift_distance);
-    __ or_(scratch2, scratch);
-    // Move down according to the exponent.
-    __ shr_cl(scratch2);
-    // Now the unsigned answer is in scratch2.  We need to move it to ecx and
-    // we may need to fix the sign.
-    Label negative;
-    __ xor_(ecx, ecx);
-    __ cmp(ecx, FieldOperand(source, HeapNumber::kExponentOffset));
-    __ j(greater, &negative, Label::kNear);
-    __ mov(ecx, scratch2);
-    __ jmp(&done, Label::kNear);
-    __ bind(&negative);
-    __ sub(ecx, scratch2);
+  // Restore registers
+  __ bind(&done);
+  if (stash_exponent_copy) {
+    __ add(esp, Immediate(kDoubleSize / 2));
   }
-  __ bind(&done);
+  __ bind(&done_no_stash);
+  if (final_result_reg.is(ecx)) __ mov(ecx, save_reg);

[Yang, 2013/07/11 12:00:27]

I think something like

if (!final_result_reg.is(result_reg)) {
  ASSERT(final_result_reg.is(ecx));
  __ mov(final_result_reg, result_reg);
}

is easier to understand.

[danno, 2013/07/11 16:00:23]

Done.

+  __ pop(save_reg);
+  __ pop(scratch1);
+  __ ret(0);
 }
 
 
 // Uses SSE2 to convert the heap number in |source| to an integer. Jumps to
 // |conversion_failure| if the heap number did not contain an int32 value.
 // Result is in ecx. Trashes ebx, xmm0, and xmm1.
 static void ConvertHeapNumberToInt32(MacroAssembler* masm,
                                      Register source,
                                      Label* conversion_failure) {
   __ movdbl(xmm0, FieldOperand(source, HeapNumber::kValueOffset));
@@ skipping 206 matching lines @@
 }
 
 
 void UnaryOpStub::GenerateHeapNumberCodeBitNot(MacroAssembler* masm,
                                                Label* slow) {
   __ mov(edx, FieldOperand(eax, HeapObject::kMapOffset));
   __ cmp(edx, masm->isolate()->factory()->heap_number_map());
   __ j(not_equal, slow);
 
   // Convert the heap number in eax to an untagged integer in ecx.
-  IntegerConvert(masm, eax, CpuFeatures::IsSupported(SSE3), slow);
+  DoubleToIStub stub(eax, ecx, HeapNumber::kValueOffset - kSmiTagSize, true);

[Yang, 2013/07/11 12:00:27]

I think you want to use (HeapNumber::kValueOffset - kHeapObjectTag). Even though
both equal to 1.

[danno, 2013/07/11 16:00:23]

Done.

+  __ call(stub.GetCode(masm->isolate()), RelocInfo::CODE_TARGET);
 
   // Do the bitwise operation and check if the result fits in a smi.
   Label try_float;
   __ not_(ecx);
   __ cmp(ecx, 0xc0000000);
   __ j(sign, &try_float, Label::kNear);
 
   // Tag the result as a smi and we're done.
   STATIC_ASSERT(kSmiTagSize == 1);
   __ lea(eax, Operand(ecx, times_2, kSmiTag));
@@ skipping 11 matching lines @@
     {
       FrameScope scope(masm, StackFrame::INTERNAL);
       // Push the original HeapNumber on the stack. The integer value can't
       // be stored since it's untagged and not in the smi range (so we can't
       // smi-tag it). We'll recalculate the value after the GC instead.
       __ push(ebx);
       __ CallRuntime(Runtime::kNumberAlloc, 0);
       // New HeapNumber is in eax.
       __ pop(edx);
     }
-    // IntegerConvert uses ebx and edi as scratch registers.
-    // This conversion won't go slow-case.
-    IntegerConvert(masm, edx, CpuFeatures::IsSupported(SSE3), slow);
+    DoubleToIStub stub(edx, ecx, HeapNumber::kValueOffset - kSmiTagSize, true);

[Yang, 2013/07/11 12:00:27]

Ditto.

[danno, 2013/07/11 16:00:23]

Done.

+    __ call(stub.GetCode(masm->isolate()), RelocInfo::CODE_TARGET);
+
     __ not_(ecx);
 
     __ bind(&heapnumber_allocated);
   }
   if (CpuFeatures::IsSupported(SSE2)) {
     CpuFeatureScope use_sse2(masm, SSE2);
     __ cvtsi2sd(xmm0, ecx);
     __ movdbl(FieldOperand(eax, HeapNumber::kValueOffset), xmm0);
   } else {
     __ push(ecx);
@@ skipping 1658 matching lines @@
   __ bind(&arg1_is_object);
   __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset));
   __ cmp(ebx, factory->heap_number_map());
   __ j(not_equal, &check_undefined_arg1);
 
   // Get the untagged integer version of the edx heap number in ecx.
   if (left_type == BinaryOpIC::INT32 && CpuFeatures::IsSupported(SSE2)) {
     CpuFeatureScope use_sse2(masm, SSE2);
     ConvertHeapNumberToInt32(masm, edx, conversion_failure);
   } else {
-    IntegerConvert(masm, edx, use_sse3, conversion_failure);
+    DoubleToIStub stub(edx, ecx, HeapNumber::kValueOffset - kSmiTagSize, true);

[Yang, 2013/07/11 12:00:27]

Ditto.

[danno, 2013/07/11 16:00:23]

Done.

+    __ call(stub.GetCode(masm->isolate()), RelocInfo::CODE_TARGET);
   }
   __ mov(edx, ecx);
 
   // Here edx has the untagged integer, eax has a Smi or a heap number.
   __ bind(&load_arg2);
 
   // Test if arg2 is a Smi.
   if (right_type == BinaryOpIC::SMI) {
     __ JumpIfNotSmi(eax, conversion_failure);
   } else {
@@ skipping 14 matching lines @@
   __ bind(&arg2_is_object);
   __ mov(ebx, FieldOperand(eax, HeapObject::kMapOffset));
   __ cmp(ebx, factory->heap_number_map());
   __ j(not_equal, &check_undefined_arg2);
   // Get the untagged integer version of the eax heap number in ecx.
 
   if (right_type == BinaryOpIC::INT32 && CpuFeatures::IsSupported(SSE2)) {
     CpuFeatureScope use_sse2(masm, SSE2);
     ConvertHeapNumberToInt32(masm, eax, conversion_failure);
   } else {
-    IntegerConvert(masm, eax, use_sse3, conversion_failure);
+    DoubleToIStub stub(eax, ecx, HeapNumber::kValueOffset - kSmiTagSize, true);

[Yang, 2013/07/11 12:00:27]

Ditto.

[danno, 2013/07/11 16:00:23]

Done.

+    __ call(stub.GetCode(masm->isolate()), RelocInfo::CODE_TARGET);
   }
 
   __ bind(&done);
   __ mov(eax, edx);
 }
 
 
 void FloatingPointHelper::LoadFloatOperand(MacroAssembler* masm,
                                            Register number) {
   Label load_smi, done;
@@ skipping 5287 matching lines @@
   __ bind(&fast_elements_case);
   GenerateCase(masm, FAST_ELEMENTS);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32