On 2014/02/18 14:08:53, jarin wrote:
lgtm with nits
https://codereview.chromium.org/163563003/diff/30001/src/harmony-math.js
File src/harmony-math.js (right):
https://codereview.chromium.org/163563003/diff/30001/src/harmony-math.js#newcode160
src/harmony-math.js:160: if (x === 0 || !NUMBER_IS_FINITE(x)) return x;
Is there any reason why you check for the corner cases for MathCbrt, but
not
for
the other functions? (runtime.cc seems to do the same checks.)
https://codereview.chromium.org/163563003/diff/30001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/163563003/diff/30001/src/runtime.cc#newcode7678
src/runtime.cc:7678: p_approx[1] = p_x[1] / 3 + 715094163; // magic
number.
Nit: could you make this more friendly to big endian?
I am thinking:
uint32_t xhigh = (uint32_t)(*(uint64_t*)&x >> 32);
uint32_t approxhigh = xhigh / 3 + 715094163;
*((uint64_t*)&approx) = (uint64_t)approxhigh << 32;
Thanks, the updated patch looks great.
https://codereview.chromium.org/163563003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
