Looking at this, I don't think the flexibility given by an arbitrary alpha
and
gamma is used anywhere, and the extra framework added isn't really likely
to be
that helpful to an implementer who wants to change them.
http://codereview.chromium.org/1032007/diff/9001/10010
File src/fast-dtoa.cc (right):
http://codereview.chromium.org/1032007/diff/9001/10010#newcode40
src/fast-dtoa.cc:40: // the result of multiplying the input number with
the cached powers-of-ten.
the range (2^32, 2^60)
or 2^32 ... 2^60
multiplying the input by a cached power of ten.
A different range might be chosen on a different platform, to optimize
digit generation, but a smaller range
requires more powers of ten to be cached.
http://codereview.chromium.org/1032007/diff/9001/10010#newcode50
src/fast-dtoa.cc:50: // representation.
Adjusts the last digit of the generated number, and screens out
generated solutions that may be inaccurate. A solution may be
inaccurate if it is outside the safe interval, or if we cannot prove
that it is closer to the input than a neighboring representable number.
http://codereview.chromium.org/1032007/diff/9001/10010#newcode58
src/fast-dtoa.cc:58: // Output: returns true on success.
returns true if the buffer is guaranteed to contain the closest
representable number to the input.
http://codereview.chromium.org/1032007/diff/9001/10010#newcode74
src/fast-dtoa.cc:74: // ]w-; w+[ (often written as "(w-; w+)")
I would write
(w-, w+) (sometimes written "]w-, w+[")
Maybe w_low and w_high would be better names?
http://codereview.chromium.org/1032007/diff/9001/10010#newcode79
src/fast-dtoa.cc:79: // By generating the digits of too_high we got the
biggest last digit.
Could you document this by saying "There are three nested intervals,
represented in our internal floating-point format that is more accurate
than 64-bit double format.
Unsafe interval, between xxxx and xxxx. A number outside this interval
cannot be the closest representable number to the input.
Safe interval, between xxxxx and xxxxx. A number inside this interval
is guaranteed to be ? Do we even use this interval?
Input interval, between w_low=xxxx and w_high=xxxx
The input number is guaranteed to be between these two values.
These intervals are approximately equal to input +/- 1/2 double ULP and
input +/- internal representation ULP.
http://codereview.chromium.org/1032007/diff/9001/10010#newcode110
src/fast-dtoa.cc:110: // Since too_low = too_high - unsafe_interval
this is equivalent too
equivalent to
What is the safe interval used for?
http://codereview.chromium.org/1032007/diff/9001/10010#newcode229
src/fast-dtoa.cc:229: // Same comments as for DigitGen (see below) but
with additional precondition:
DigitGen does (approximately) nothing now, and could easily be recreated
if someone wanted a general max_exponent and min_exponent for the range.
I think DigitGen should just be deleted from the file, and its comments
put here. This could be renamed DigitGen.
http://codereview.chromium.org/1032007/diff/9001/10010#newcode375
src/fast-dtoa.cc:375: bool DigitGen(DiyFp low,
This function does nothing, and doesn't really help anyone except a
hypothetical library extender, and doesn't even really help them much.
http://codereview.chromium.org/1032007/diff/9001/10010#newcode404
src/fast-dtoa.cc:404: // Returns true if it succeeds, otherwise the
result can not be trusted.
cannot
http://codereview.chromium.org/1032007/diff/9001/10010#newcode409
src/fast-dtoa.cc:409: // 0.099999999999 instead of 0.1.
This is inaccurate - if 0.099999999999 is closer than 0.1, then
0.09999999999 is returned. You mean no 0.10000 instead of 0.1.
http://codereview.chromium.org/1032007/diff/9001/10010#newcode415
src/fast-dtoa.cc:415: // boundary_minus and boundary_plus are the
boundaries between v and its
its closest representable neighbors
http://codereview.chromium.org/1032007/diff/9001/10010#newcode417
src/fast-dtoa.cc:417: // will round to v when read as double.
when converted to double?
http://codereview.chromium.org/1032007/diff/9001/10006
File src/grisu3.cc (left):
http://codereview.chromium.org/1032007/diff/9001/10006#oldcode39
src/grisu3.cc:39: template <int alpha = -60, int gamma = -32>
I thought you were getting rid of the template?
http://codereview.chromium.org/1032007/diff/9001/10004
File src/grisu3.h (left):
http://codereview.chromium.org/1032007/diff/9001/10004#oldcode39
src/grisu3.h:39: // v must satisfy v != 0 and it must not be Infinity
or NaN.
v must not be (positive or negative) zero, Infinity, or NaN.
http://codereview.chromium.org/1032007
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
