Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(688)

Issue 305403002: fdlibm: reduce accuracy slightly in favor of performance. (Closed)

Created:
6 years, 6 months ago by Yang
Modified:
6 years, 4 months ago
Reviewers:
Raymond Toy
CC:
v8-dev
Visibility:
Public.

Description

fdlibm: reduce accuracy slightly in favor of performance.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -100 lines) Patch
M src/math.js View 14 chunks +36 lines, -85 lines 1 comment Download
M src/rempio2.cc View 2 chunks +2 lines, -6 lines 0 comments Download
M src/runtime.cc View 1 chunk +4 lines, -5 lines 0 comments Download
M test/mjsunit/sin-cos.js View 3 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
Raymond Toy
6 years, 6 months ago (2014-06-02 16:55:53 UTC) #1
I don't see that saving 2ms (from 17) here is worth the loss of accuracy.  

Accurate reduction doesn't come for free and this is much better than the
current version for medium to large arguments.

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

https://codereview.chromium.org/305403002/diff/1/src/math.js#newcode199
src/math.js:199: y = x - pio2_1 - pio2_2 - pio2_2t;
I don't think this is computing the same values as before.

What happened to the if statement? The value of y0 is different on the two
branches.  Can you prove that y is the same value as y0 on both branches.

And I think you need to be careful about the order of operations in computing y.
 It's not clear to me that the compiler won't notice that pio2_1, pio2_2, and
pio2_2t are constants that the compiler won't just coalesce into one constant,
thereby destroying all of the extra bits you actually want to use.

Powered by Google App Engine
This is Rietveld 408576698