ARM: Implement correct rounding in the lithium codegenerator.

src/arm/lithium-codegen-arm.cc

Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 49124497fba7dc5566a77950321e6858ff40f0df..b49981288559817d32a4f5fa3d8e03f96ab95d20 100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -2861,8 +2861,106 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
 void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
   DoubleRegister input = ToDoubleRegister(instr->InputAt(0));
   Register result = ToRegister(instr->result());
-  Register scratch1 = scratch0();
-  Register scratch2 = result;
+  Register scratch1 = ToRegister(instr->TempAt(0));
+  Register scratch2 = scratch0();
+  Register scratch3 = result;
+
+  // Test for numbers with a decimal fraction of 0.5 first.
+  // These cannot be rounded with kRoundToNearest, because it rounds
+  // ties to even numbers instead of to the higher one, as required
+  // by the ECMA standard.
+
+  // Extract exponent bits. scratch1 holds the upper word
+  // of the double and scratch2 the lower word.
+  __ vmov(scratch2, scratch1, input);
+  __ ubfx(scratch3,
+          scratch1,
+          HeapNumber::kExponentShift,
+          HeapNumber::kExponentBits);
+
+  // Unbias exponent.
+  __ sub(scratch3, scratch3, Operand(HeapNumber::kExponentBias));
+
+  // Deoptimize if value is special.
+  __ cmp(scratch3, Operand(1024));

[Lasse Reichstein, 2011/04/14 19:09:04]

You can deoptimize for all exponents above or equal to 31 (or 32, if you don't
want to miss -(2^31)). The value won't fit in an int32 in any case.

+  DeoptimizeIf(eq, instr->environment());
+
+  Label lower_word, one_half, test, normal_case, adjust_sign_on_zero, done;
+
+  // If the exponent is -1 jump to special check for 0.5.
+  __ cmp(scratch3, Operand(-1));
+  __ b(eq, &one_half);
+
+  // If the exponent is smaller than -1, the result is +0
+  // or -0.
+  __ mov(result, Operand(0), LeaveCC, lt);
+  __ b(lt, &adjust_sign_on_zero);

[Lasse Reichstein, 2011/04/14 19:09:04]

You can jump to after the zero-test at adjust_sign_on_zero.
Or just jump directly to done if we don't care about -0's.

+
+  // If the exponent is greater or equal to HeapNumber::kMantissaBits then
+  // there is no fractional part.
+  __ cmp(scratch3, Operand(HeapNumber::kMantissaBits));

[Lasse Reichstein, 2011/04/14 19:09:04]

If you bailed out for exponent >= 31, this isn't needed.

+  __ b(ge, &normal_case);
+
+  // Exponent is greater or equal zero and below HeapNumber::kMantissaBits.

[Søren Thygesen Gjesse, 2011/04/15 07:55:35]

equal zero -> equal to zero

+  // Check if the bit we want to test is in the upper or lower word.

[Lasse Reichstein, 2011/04/14 19:09:04]

The bit we need to test is the one with value 0.5, right?
(I.e., more explanation of what we are trying to do here)

+  __ cmp(scratch3, Operand(HeapNumber::kMantissaBitsInTopWord - 1));

[Lasse Reichstein, 2011/04/14 19:09:04]

You might be able to save a few instructions here by doing:
rsb(scratch3, scratch3, Operand(HeapNumber::kMantissaBitsInTopWord - 1))
mov(scratch1, scratch2, LeaveCC, ge);
b(ge, &test);
// lower_word here.
add(scratch3, scratch3, Operand(HeapNumber::kMantissaBits -
HeapNumber::kMantissaBitsInTopWord));

+  __ b(gt, &lower_word);
+
+  // If the first fractional bit is in the upper word and the fractional part
+  // is 0.5 then the lower word of the double must be all zeroes.
+  __ cmp(scratch2, Operand(0));
+  __ b(ne, &normal_case);
+
+  // Calculate the bit position of the 1-bit we are looking for. All lower
+  // bits must be zero.
+  __ rsb(scratch3, scratch3, Operand(HeapNumber::kMantissaBitsInTopWord - 1));
+  __ jmp(&test);
+
+  __ bind(&lower_word);
+  // Calculate the bit position of the 1-bit we are looking for. All lower
+  // bits must be zero. Copy lower word into higher word register for the
+  // test code below.
+  __ rsb(scratch3, scratch3, Operand(HeapNumber::kMantissaBits - 1));
+  __ mov(scratch1, scratch2);
+
+  __ bind(&test);
+  // Test the bit.
+  __ mov(scratch2, Operand(1));
+  __ mov(scratch3, Operand(scratch2, LSL, scratch3));
+  __ tst(scratch1, scratch3);
+  __ b(eq, &normal_case);
+
+  // Test the rest of the mantissa in the high word.
+  __ sub(scratch3, scratch3, Operand(1));
+  __ tst(scratch1, scratch3);
+  __ b(ne, &normal_case);
+
+  // We have to round to the higher number, so we add 0.5 and jump to
+  // the normal case.

[Lasse Reichstein, 2011/04/14 19:09:04]

If you are going to add 0.5 and round in half the cases, you might as well just
add 0.5 and round down for all numbers except [-0.5..0.5[.

The only cases where adding 0.5 and rounding down does not work is when the 0.5
bit is just outside the range of bits in the double, so adding it will lose
precision (i.e., (+/-) 0.49999999999999994 and [2^52..2^53-1], and the latter is
outside the int32 range we are aiming for).
I.e., you only need the special case for -0.5 and for exponent < -1 (the
negative ones round to -0 and the positive ones doesn't all work with
floor(x+0.5)).

+  ExternalReference one_half_adr = ExternalReference::address_of_one_half();
+  __ mov(scratch1, Operand(one_half_adr));
+  __ vldr(double_scratch0(), MemOperand(scratch1));
+  __ vadd(input, input, double_scratch0());
+  __ jmp(&normal_case);
+
+  __ bind(&one_half);
+  // Check for the special case of +0.5 and -0.5: The mantissa
+  // bits must be all zero.
+  __ tst(scratch1, Operand(HeapNumber::kMantissaMask));
+  __ b(ne, &normal_case);
+  __ cmp(scratch2, Operand(0));
+  __ b(ne, &normal_case);
+  __ tst(scratch1, Operand(HeapNumber::kSignMask));
+  __ mov(result, Operand(1), LeaveCC, eq);
+  __ b(&done, eq);
+  if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
+    DeoptimizeIf(ne, instr->environment());
+  } else {
+    __ mov(result, Operand(0));
+    __ jmp(&done);
+  }
+
+  __ bind(&normal_case);
   __ EmitVFPTruncate(kRoundToNearest,
                      double_scratch0().low(),
                      input,
@@ -2871,16 +2969,16 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
   DeoptimizeIf(ne, instr->environment());
   __ vmov(result, double_scratch0().low());
 
+  __ bind(&adjust_sign_on_zero);
   if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
     // Test for -0.
-    Label done;
     __ cmp(result, Operand(0));
     __ b(ne, &done);
     __ vmov(scratch1, input.high());
     __ tst(scratch1, Operand(HeapNumber::kSignMask));
     DeoptimizeIf(ne, instr->environment());
-    __ bind(&done);
   }
+  __ bind(&done);
 }