Drive by...

LGTM


http://codereview.chromium.org/5720005/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/5720005/diff/1/src/arm/code-stubs-arm.cc#newcode2276
src/arm/code-stubs-arm.cc:2276: // Two uint_32's, a pointer, and an
untagged double.
Not your error, but the type is called uint32_t.

http://codereview.chromium.org/5720005/diff/1/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/5720005/diff/1/src/heap.h#newcode2051
src/heap.h:2051: static const int kElementSize = sizeof (Element);
There should really be some constants here that you can use in the
architecture dependent files.  All that 3 * kIntSize is rather nasty.
If you put the pointer last the offsets will be independent of 32 or 64
bit.

http://codereview.chromium.org/5720005/diff/1/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

http://codereview.chromium.org/5720005/diff/1/src/ia32/code-stubs-ia32.cc#newcode2566
src/ia32/code-stubs-ia32.cc:2566: __ j(zero, &allocate_cached_answer);
// The answer is cached as untagged.
"is cached, but only as untagged".

http://codereview.chromium.org/5720005/diff/1/src/ia32/code-stubs-ia32.cc#newcode2753
src/ia32/code-stubs-ia32.cc:2753: CHECK_EQ(20, elem2_start -
elem_start);  // Two uint_32's and a pointer.
Comment is now wrong and type has wrong name.

http://codereview.chromium.org/5720005/diff/1/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

http://codereview.chromium.org/5720005/diff/1/src/x64/code-stubs-x64.cc#newcode1073
src/x64/code-stubs-x64.cc:1073: // Two uint_32's and a pointer per
element.
Wrong type name and out of date comment.

http://codereview.chromium.org/5720005/diff/1/src/x64/code-stubs-x64.cc#newcode1081
src/x64/code-stubs-x64.cc:1081: // Find the address of the rcx'th entry
in the cache, i.e., &rax[rcx*16].
Out of date comment.

http://codereview.chromium.org/5720005/diff/1/src/x64/code-stubs-x64.cc#newcode1085
src/x64/code-stubs-x64.cc:1085: Label cache_miss;
Surely this is still within range of a NearLabel.

http://codereview.chromium.org/5720005/diff/1/src/x64/code-stubs-x64.cc#newcode1093
src/x64/code-stubs-x64.cc:1093: __ j(zero, &allocate_cached_answer);  //
The answer is cached as untagged.
Comment could be clearer again.

http://codereview.chromium.org/5720005/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to