Fix bug in X64 RSet code. Optimize IA32 version.

src/ia32/macro-assembler-ia32.cc

Index: src/ia32/macro-assembler-ia32.cc
diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc
index de0ef8ece9b27c84f0eab2b5d08fb80dbcde1ae1..246350b081a7b772244744c2436196975e82dae4 100644
--- a/src/ia32/macro-assembler-ia32.cc
+++ b/src/ia32/macro-assembler-ia32.cc
@@ -70,25 +70,26 @@ static void RecordWriteHelper(MacroAssembler* masm,
   // Adjust 'addr' to be relative to the start of the extra remembered set

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

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.

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

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

   // and the page address in 'object' to be the address of the extra
   // remembered set.
-  masm->sub(Operand(addr), Immediate(Page::kPageSize / kPointerSize));
-  // Load the array length into 'scratch' and multiply by four to get the
-  // size in bytes of the elements.
+
+  // Find the length of the FixedArray.
   masm->mov(scratch, Operand(object, Page::kObjectStartOffset
                                      + FixedArray::kLengthOffset));
-  masm->shl(scratch, kObjectAlignmentBits);
-  // Add the page header, array header, and array body size to the page
-  // address.
-  masm->add(Operand(object), Immediate(Page::kObjectStartOffset
-                                       + FixedArray::kHeaderSize));
-  masm->add(object, Operand(scratch));
-
 
+  // The RSet extension area lies after the FixedArray, i.e.,

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

how about "after the FixedArray, at"

+  // at
+  //   object + kObjectStartOffset + FixedArray::kHeaderSize + 8 * scratch

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

Surely not 8?

+  // Make object point to (size of normal RSet) before that, so that we
+  // can address the bit directly with addr.
+  masm->lea(object,
+            Operand(object, scratch, times_pointer_size,
+                    Page::kObjectStartOffset + FixedArray::kHeaderSize
+                        - Page::kRSetEndOffset));
   // NOTE: For now, we use the bit-test-and-set (bts) x86 instruction
   // to limit code size. We should probably evaluate this decision by
   // measuring the performance of an equivalent implementation using
   // "simpler" instructions
   masm->bind(&fast);
-  masm->bts(Operand(object, 0), addr);
+  masm->bts(Operand(object, Page::kRSetOffset), addr);
 }