Increase precision when finding the remainder after division by pi/2.

Increase precision when finding the remainder after division by pi/2.

R=jkummerow@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=17933

[Yang, 2013-11-19 16:43:11]

Please take a look.

[Jakob Kummerow, 2013-11-19 17:49:53]

LGTM with a comment.

https://codereview.chromium.org/66703005/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/66703005/diff/1/src/runtime.cc#newcode6892
src/runtime.cc:6892: uint8_t* bytes = reinterpret_cast<uint8_t*>(&result);
Are you sure this compiles? AFAIK this is what we have BitCast for. (You'll
probably want to start with an uint8_t[8] and then BitCast to double.)

[Sven Panne, 2013-11-20 07:38:34]

DBC

https://codereview.chromium.org/66703005/diff/1/src/math.js
File src/math.js (right):

https://codereview.chromium.org/66703005/diff/1/src/math.js#newcode222
src/math.js:222: var inverse_pi_half = %HeapNumberFromHex("83c8c96d305fe43f");
Why do we need a new %Foo? Our scanner should be able to produce the bit pattern
you want directly from a floating point literal. If not, fix the scanner instead
of introducing a new %Foo.

https://codereview.chromium.org/66703005/diff/1/test/mjsunit/sin-cos.js
File test/mjsunit/sin-cos.js (right):

https://codereview.chromium.org/66703005/diff/1/test/mjsunit/sin-cos.js#newco...
test/mjsunit/sin-cos.js:148: function assertError(expected, x, epsilon) {
Can we move this to mjsunit.js under a better name, e.g.
assertEqualsDelta(expected, actual, delta, opt_message)? Or just assertEquals
and dispatch on the third argument (message or delta). See
http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertEquals(double,
double, double)

[Yang, 2013-11-20 09:05:46]

On 2013/11/20 07:38:34, Sven Panne wrote:
> DBC
> 
> https://codereview.chromium.org/66703005/diff/1/src/math.js
> File src/math.js (right):
> 
> https://codereview.chromium.org/66703005/diff/1/src/math.js#newcode222
> src/math.js:222: var inverse_pi_half = %HeapNumberFromHex("83c8c96d305fe43f");
> Why do we need a new %Foo? Our scanner should be able to produce the bit
pattern
> you want directly from a floating point literal. If not, fix the scanner
instead
> of introducing a new %Foo.
> 
> https://codereview.chromium.org/66703005/diff/1/test/mjsunit/sin-cos.js
> File test/mjsunit/sin-cos.js (right):
> 
>
https://codereview.chromium.org/66703005/diff/1/test/mjsunit/sin-cos.js#newco...
> test/mjsunit/sin-cos.js:148: function assertError(expected, x, epsilon) {
> Can we move this to mjsunit.js under a better name, e.g.
> assertEqualsDelta(expected, actual, delta, opt_message)? Or just assertEquals
> and dispatch on the third argument (message or delta). See
>
http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertEquals%28double,
> double, double)

addressed comments.

[Yang, 2013-11-20 10:56:23]

On 2013/11/20 09:05:46, Yang wrote:
> On 2013/11/20 07:38:34, Sven Panne wrote:
> > DBC
> > 
> > https://codereview.chromium.org/66703005/diff/1/src/math.js
> > File src/math.js (right):
> > 
> > https://codereview.chromium.org/66703005/diff/1/src/math.js#newcode222
> > src/math.js:222: var inverse_pi_half =
%HeapNumberFromHex("83c8c96d305fe43f");
> > Why do we need a new %Foo? Our scanner should be able to produce the bit
> pattern
> > you want directly from a floating point literal. If not, fix the scanner
> instead
> > of introducing a new %Foo.
> > 
> > https://codereview.chromium.org/66703005/diff/1/test/mjsunit/sin-cos.js
> > File test/mjsunit/sin-cos.js (right):
> > 
> >
>
https://codereview.chromium.org/66703005/diff/1/test/mjsunit/sin-cos.js#newco...
> > test/mjsunit/sin-cos.js:148: function assertError(expected, x, epsilon) {
> > Can we move this to mjsunit.js under a better name, e.g.
> > assertEqualsDelta(expected, actual, delta, opt_message)? Or just
assertEquals
> > and dispatch on the third argument (message or delta). See
> >
>
http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertEquals%2528d...,
> > double, double)
> 
> addressed comments.

Added some further improvements.

[Jakob Kummerow, 2013-11-20 12:41:42]

lgtm

[Yang, 2013-11-20 15:04:45]

Committed patchset #4 manually as r17933 (presubmit successful).