I have addressed all your concerns. Please have another look.
http://codereview.chromium.org/619005/diff/10032/10037 File src/cached_powers.h (right): http://codereview.chromium.org/619005/diff/10032/10037#newcode42 src/cached_powers.h:42: // The following defines "implement" the interface between this file and the On 2010/03/09 12:21:57, William Hesse wrote:
Avoid using quotes to qualify or weaken words. Just say implement.
Done. http://codereview.chromium.org/619005/diff/10032/10037#newcode45 src/cached_powers.h:45: // GRISU_CACHE_NAME(i) contains GRISU_CACHE_NAME(1) where only every every 'i'th On 2010/03/09 12:21:57, William Hesse wrote:
every every
Done. http://codereview.chromium.org/619005/diff/10032/10037#newcode48 src/cached_powers.h:48: // The higher 'i' is the less elements we use. On 2010/03/09 12:21:57, William Hesse wrote:
fewer elements
Done. http://codereview.chromium.org/619005/diff/10032/10047 File src/checks.h (right): http://codereview.chromium.org/619005/diff/10032/10047#newcode105 src/checks.h:105: On 2010/03/09 12:21:57, William Hesse wrote:
Make sure this works on all platforms, without introducing compilation
failures,
by checking it in separately. Or else drop it. We have had troubles
with this
in the past.
Removed. http://codereview.chromium.org/619005/diff/10032/10038 File src/diy_fp.h (right): http://codereview.chromium.org/619005/diff/10032/10038#newcode34 src/diy_fp.h:34: // This "Do It Yourself Floating Point" class implements a 64bit significand/int On 2010/03/09 12:21:57, William Hesse wrote:
implements a floating point number with a uint64 significand and an
int
exponent. Normalized DiyFp numbers will have the top bit of the
significand
set, and multiplication and subtraction do not normalize their
results. Done. http://codereview.chromium.org/619005/diff/10032/10038#newcode48 src/diy_fp.h:48: // than other. The result will not be normalized. On 2010/03/09 12:21:57, William Hesse wrote:
and the significand of this must be bigger than the significand of
other. Done. http://codereview.chromium.org/619005/diff/10032/10035 File src/grisu3.cc (right): http://codereview.chromium.org/619005/diff/10032/10035#newcode56 src/grisu3.cc:56: static bool RoundWeed(char* buffer, int len, uint64_t wp_W, uint64_t Delta, On 2010/03/09 13:22:05, William Hesse wrote:
Comment these with a statement describing what they do.
Done. http://codereview.chromium.org/619005/diff/10032/10035#newcode69 src/grisu3.cc:69: // boundary_minus and boundary_plus are the boundaries between w and its On 2010/03/09 13:22:05, William Hesse wrote:
Don't mix v and w in this comment. Probably use v only. Don't use y
- just say
x again. You could say "any number strictly between b_m and b_p will
round to v
when converted to double.
Done. http://codereview.chromium.org/619005/diff/10032/10035#newcode89 src/grisu3.cc:89: // In fact: scaled_w - w*10^k < 1ulp (unit in the last place) of scaled_w. On 2010/03/09 13:22:05, William Hesse wrote:
Can't this be exactly 1 lulp off? If cached_power is off by 1/2 lulp,
and the
multiplication is off by 1/2 lulp?
I have proved in the paper that the error is still less than 1 ulp. Should I add more comments? http://codereview.chromium.org/619005/diff/10032/10035#newcode106 src/grisu3.cc:106: // comma-number it will be updated. On 2010/03/09 13:22:05, William Hesse wrote:
What is a comma-number?
Done. http://codereview.chromium.org/619005/diff/10032/10040 File src/powers_ten.h (right): http://codereview.chromium.org/619005/diff/10032/10040#newcode4 src/powers_ten.h:4: On 2010/03/09 12:21:57, William Hesse wrote:
Add comment: When this header file is used by xxxx.h, only one of the
arrays
below is accessed, the smallest array that will work with the given
values of
alpha and gamma. The intention is that only this array will be linked
into the
compiled code by the compiler.
added comments. http://codereview.chromium.org/619005/diff/10032/10044 File test/cctest/test-double.cc (right): http://codereview.chromium.org/619005/diff/10032/10044#newcode31 test/cctest/test-double.cc:31: // The last 52 bits + 1 hidden bit a position 52. On 2010/03/09 12:21:57, William Hesse wrote:
The 52 mantissa bits, plus the implicit 1 in bit 52, as a UINT64.
Done. http://codereview.chromium.org/619005/diff/10032/10044#newcode94 test/cctest/test-double.cc:94: CHECK(!Double(1.7976931348623157e308).IsSpecial()); On 2010/03/09 12:21:57, William Hesse wrote:
What about negative numbers?
Done. http://codereview.chromium.org/619005/diff/10032/10046 File tools/generate-ten-powers.scm (right): http://codereview.chromium.org/619005/diff/10032/10046#newcode30 tools/generate-ten-powers.scm:30: ;; bigloo -static-bigloo -o generate-ten-powers generate-ten-powers.scm On 2010/03/09 12:21:57, William Hesse wrote:
Mention that this is a scheme script, and that bigloo is a scheme
compiler? Done. http://codereview.chromium.org/619005 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
