Revision: 4180
Author: [email protected]
Date: Thu Mar 18 06:00:57 2010
Log: Improve Math.round(). Fix the bug in r4146. Further improve performance by checking the exponent instead of comparing doubles. Add several tests for numbers near the limits of SMI and several tests from WebKit.

Review URL: http://codereview.chromium.org/1008004
http://code.google.com/p/v8/source/detail?r=4180

Modified:
 /branches/bleeding_edge/src/math.js
 /branches/bleeding_edge/src/objects-inl.h
 /branches/bleeding_edge/src/objects.h
 /branches/bleeding_edge/src/runtime.cc
 /branches/bleeding_edge/src/runtime.h
 /branches/bleeding_edge/test/mjsunit/math-round.js

=======================================
--- /branches/bleeding_edge/src/math.js Thu Mar 11 01:49:47 2010
+++ /branches/bleeding_edge/src/math.js Thu Mar 18 06:00:57 2010
@@ -171,7 +171,7 @@
 // ECMA 262 - 15.8.2.15
 function MathRound(x) {
   if (!IS_NUMBER(x)) x = ToNumber(x);
-  return %Math_round(x);
+  return %RoundNumber(x);
 }

 // ECMA 262 - 15.8.2.16
=======================================
--- /branches/bleeding_edge/src/objects-inl.h   Thu Mar 11 08:24:31 2010
+++ /branches/bleeding_edge/src/objects-inl.h   Thu Mar 18 06:00:57 2010
@@ -1119,6 +1119,17 @@
 void HeapNumber::set_value(double value) {
   WRITE_DOUBLE_FIELD(this, kValueOffset, value);
 }
+
+
+int HeapNumber::get_exponent() {
+  return ((READ_INT_FIELD(this, kExponentOffset) & kExponentMask) >>
+          kExponentShift) - kExponentBias;
+}
+
+
+int HeapNumber::get_sign() {
+  return READ_INT_FIELD(this, kExponentOffset) & kSignMask;
+}


 ACCESSORS(JSObject, properties, FixedArray, kPropertiesOffset)
=======================================
--- /branches/bleeding_edge/src/objects.h       Fri Mar 12 02:20:01 2010
+++ /branches/bleeding_edge/src/objects.h       Thu Mar 18 06:00:57 2010
@@ -1094,6 +1094,9 @@
   void HeapNumberVerify();
 #endif

+  inline int get_exponent();
+  inline int get_sign();
+
   // Layout description.
   static const int kValueOffset = HeapObject::kHeaderSize;
// IEEE doubles are two 32 bit words. The first is just mantissa, the second
=======================================
--- /branches/bleeding_edge/src/runtime.cc      Thu Mar 18 02:38:30 2010
+++ /branches/bleeding_edge/src/runtime.cc      Thu Mar 18 06:00:57 2010
@@ -5350,16 +5350,38 @@
 }


-static Object* Runtime_Math_round(Arguments args) {
+static Object* Runtime_RoundNumber(Arguments args) {
   NoHandleAllocation ha;
   ASSERT(args.length() == 1);
   Counters::math_round.Increment();

-  CONVERT_DOUBLE_CHECKED(x, args[0]);
-  if (signbit(x) && x >= -0.5) return Heap::minus_zero_value();
-  double integer = ceil(x);
-  if (integer - x > 0.5) { integer -= 1.0; }
-  return Heap::NumberFromDouble(integer);
+  if (!args[0]->IsHeapNumber()) {
+    // Must be smi. Return the argument unchanged for all the other types
+    // to make fuzz-natives test happy.
+    return args[0];
+  }
+
+  HeapNumber* number = reinterpret_cast<HeapNumber*>(args[0]);
+
+  double value = number->value();
+  int exponent = number->get_exponent();
+  int sign = number->get_sign();
+
+ // We compare with kSmiValueSize - 3 because (2^30 - 0.1) has exponent 29 and
+  // should be rounded to 2^30, which is not smi.
+  if (!sign && exponent <= kSmiValueSize - 3) {
+    return Smi::FromInt(static_cast<int>(value + 0.5));
+  }
+
+ // If the magnitude is big enough, there's no place for fraction part. If we
+  // try to add 0.5 to this number, 1.0 will be added instead.
+  if (exponent >= 52) {
+    return number;
+  }
+
+  if (sign && value >= -0.5) return Heap::minus_zero_value();
+
+  return Heap::NumberFromDouble(floor(value + 0.5));
 }


=======================================
--- /branches/bleeding_edge/src/runtime.h       Mon Mar 15 14:06:51 2010
+++ /branches/bleeding_edge/src/runtime.h       Thu Mar 18 06:00:57 2010
@@ -145,7 +145,7 @@
   F(Math_log, 1, 1) \
   F(Math_pow, 2, 1) \
   F(Math_pow_cfunction, 2, 1) \
-  F(Math_round, 1, 1) \
+  F(RoundNumber, 1, 1) \
   F(Math_sin, 1, 1) \
   F(Math_sqrt, 1, 1) \
   F(Math_tan, 1, 1) \
=======================================
--- /branches/bleeding_edge/test/mjsunit/math-round.js Tue Feb 2 01:14:22 2010 +++ /branches/bleeding_edge/test/mjsunit/math-round.js Thu Mar 18 06:00:57 2010
@@ -50,3 +50,52 @@
 assertEquals(-9007199254740991, Math.round(-9007199254740991));
 assertEquals(Number.MAX_VALUE, Math.round(Number.MAX_VALUE));
 assertEquals(-Number.MAX_VALUE, Math.round(-Number.MAX_VALUE));
+
+assertEquals(536870911, Math.round(536870910.5));
+assertEquals(536870911, Math.round(536870911));
+assertEquals(536870911, Math.round(536870911.4));
+assertEquals(536870912, Math.round(536870911.5));
+assertEquals(536870912, Math.round(536870912));
+assertEquals(536870912, Math.round(536870912.4));
+assertEquals(536870913, Math.round(536870912.5));
+assertEquals(536870913, Math.round(536870913));
+assertEquals(536870913, Math.round(536870913.4));
+assertEquals(1073741823, Math.round(1073741822.5));
+assertEquals(1073741823, Math.round(1073741823));
+assertEquals(1073741823, Math.round(1073741823.4));
+assertEquals(1073741824, Math.round(1073741823.5));
+assertEquals(1073741824, Math.round(1073741824));
+assertEquals(1073741824, Math.round(1073741824.4));
+assertEquals(1073741825, Math.round(1073741824.5));
+assertEquals(2147483647, Math.round(2147483646.5));
+assertEquals(2147483647, Math.round(2147483647));
+assertEquals(2147483647, Math.round(2147483647.4));
+assertEquals(2147483648, Math.round(2147483647.5));
+assertEquals(2147483648, Math.round(2147483648));
+assertEquals(2147483648, Math.round(2147483648.4));
+assertEquals(2147483649, Math.round(2147483648.5));
+
+// Tests based on WebKit LayoutTests
+
+assertEquals(0, Math.round(0.4));
+assertEquals(-0, Math.round(-0.4));
+assertEquals(-0, Math.round(-0.5));
+assertEquals(1, Math.round(0.6));
+assertEquals(-1, Math.round(-0.6));
+assertEquals(2, Math.round(1.5));
+assertEquals(2, Math.round(1.6));
+assertEquals(-2, Math.round(-1.6));
+assertEquals(8640000000000000, Math.round(8640000000000000));
+assertEquals(8640000000000001, Math.round(8640000000000001));
+assertEquals(8640000000000002, Math.round(8640000000000002));
+assertEquals(9007199254740990, Math.round(9007199254740990));
+assertEquals(9007199254740991, Math.round(9007199254740991));
+assertEquals(1.7976931348623157e+308, Math.round(1.7976931348623157e+308));
+assertEquals(-8640000000000000, Math.round(-8640000000000000));
+assertEquals(-8640000000000001, Math.round(-8640000000000001));
+assertEquals(-8640000000000002, Math.round(-8640000000000002));
+assertEquals(-9007199254740990, Math.round(-9007199254740990));
+assertEquals(-9007199254740991, Math.round(-9007199254740991));
+assertEquals(-1.7976931348623157e+308, Math.round(-1.7976931348623157e+308));
+assertEquals(Infinity, Math.round(Infinity));
+assertEquals(-Infinity, Math.round(-Infinity));

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to