Fix bug in X64 RSet code. Optimize IA32 version.

Fix bug in X64 RSet code. Optimize IA32 version.

[Lasse Reichstein, 2009-08-05 09:51:56]

Smallish review.

[William Hesse, 2009-08-05 10:17:50]

LGTM.  Are the Mozilla tests verified?

http://codereview.chromium.org/162001/diff/1/2
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/162001/diff/1/2#newcode70
Line 70: // Adjust 'addr' to be relative to the start of the extra remembered
set
This comment is no longer right.  I would comment
"Store the length of the fixed array in object."
and 
"Let object be the starting address of the extra remembered set, minus
kRSetOffset and minus an offset of the remembered set bits which are stored in
the first part of the remembered set, but still counted in addr.
The two offsets add up to kRSetEndOffset.

http://codereview.chromium.org/162001/diff/1/2#newcode78
Line 78: // The RSet extension area lies after the FixedArray, i.e.,
how about "after the FixedArray, at"

http://codereview.chromium.org/162001/diff/1/3
File src/spaces-inl.h (right):

http://codereview.chromium.org/162001/diff/1/3#newcode131
Line 131: 
Rather than saying "FixedArray" everywhere in comments and identifier names,
could we just say "the large object"?  And mention once that it is a FixedArray?

[Kevin Millikin (Chromium), 2009-08-05 10:26:15]

LGTM.

http://codereview.chromium.org/162001/diff/1/2
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/162001/diff/1/2#newcode59
Line 59: masm->and_(object, ~Page::kPageAlignmentMask);
This whole function is probably clearer if we introduce a symbolic alias for
'object'.  Something like 'base', 'start', 'page_start'?

http://codereview.chromium.org/162001/diff/1/2#newcode70
Line 70: // Adjust 'addr' to be relative to the start of the extra remembered
set
On 2009/08/05 10:17:50, William Hesse wrote:
> This comment is no longer right.  I would comment
> "Store the length of the fixed array in object."
> and 
> "Let object be the starting address of the extra remembered set, minus
> kRSetOffset and minus an offset of the remembered set bits which are stored in
> the first part of the remembered set, but still counted in addr.
> The two offsets add up to kRSetEndOffset.

I would ditch the whole comment and use symbolic aliases for the register names
to make it more clear.

http://codereview.chromium.org/162001/diff/1/2#newcode80
Line 80: //   object + kObjectStartOffset + FixedArray::kHeaderSize + 8 *
scratch
Surely not 8?

[William Hesse, 2009-08-05 10:36:15]

LGTM.  Are the Mozilla tests verified?