LGTM!

http://codereview.chromium.org/3144002/diff/1/8
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/3144002/diff/1/8#newcode454
src/ia32/ic-ia32.cc:454: // Loads an indexed element from a fast case
array.
Maybe update the comment (either here or in the header file) with
information about the use of the not_fast_array label?

http://codereview.chromium.org/3144002/diff/1/8#newcode568
src/ia32/ic-ia32.cc:568: 1 << Map::kHasFastElements);
Should we update the name of the bit to HasFastElementsForReading to
reflect the new meaning?

Maybe that is not needed with the new explanation to the general
HasFastElements method, but it might clearify the code generators to be
explicit about it in the name of the bit.

http://codereview.chromium.org/3144002/diff/1/13
File src/objects-inl.h (right):

http://codereview.chromium.org/3144002/diff/1/13#newcode1401
src/objects-inl.h:1401: ASSERT(map_word().ToMap() !=
Heap::raw_unchecked_fixed_cow_array_map());
Just use 'map()' in all of these asserts?

Are these used during GC so that you need to use
raw_unchecked_fixed_cow_array_map or could you just use
fixed_cow_array_map()?

http://codereview.chromium.org/3144002/show

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

Reply via email to