LGTM

http://codereview.chromium.org/7113012/diff/1/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/7113012/diff/1/src/arm/macro-assembler-arm.cc#newcode3142
src/arm/macro-assembler-arm.cc:3142: void
MacroAssembler::HasColour(Register object,
I think we generally use the Bowdlerized US English spelling, however
much it might hurt.
Variables too.

http://codereview.chromium.org/7113012/diff/1/src/arm/macro-assembler-arm.cc#newcode3150
src/arm/macro-assembler-arm.cc:3150: MarkBits(object, bitmap_scratch,
mask_scratch);
Could MarkBits be named something that sounds less imperative? I get
confused as to whether it actually marks any bits, or just finds the
address/mask of the mark-bits.
Something like GetMarkBitAddressAndMask would perhaps be too long, but
more readable.

http://codereview.chromium.org/7113012/diff/1/src/arm/macro-assembler-arm.cc#newcode3158
src/arm/macro-assembler-arm.cc:3158: b(eq, &word_boundary);
Should this be optimized for speed or size?
If speed, you could probably detect the case where the color overlaps a
word boundary earlier (mask is negative) and have a faster case that
tests both bits in one test if it doesn't.

http://codereview.chromium.org/7113012/diff/1/src/arm/macro-assembler-arm.h
File src/arm/macro-assembler-arm.h (right):

http://codereview.chromium.org/7113012/diff/1/src/arm/macro-assembler-arm.h#newcode1145
src/arm/macro-assembler-arm.h:1145: // the position of the first bit.
Uses ecx as scratch and leaves addr_reg
ecx?

http://codereview.chromium.org/7113012/

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

Reply via email to