Revision: 2626 Author: [email protected] Date: Wed Aug 5 04:08:24 2009 Log: Fix bug in X64 RSet code. Optimize IA32 version.
Review URL: http://codereview.chromium.org/162001 http://code.google.com/p/v8/source/detail?r=2626 Modified: /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc /branches/bleeding_edge/src/spaces-inl.h /branches/bleeding_edge/src/x64/macro-assembler-x64.cc /branches/bleeding_edge/test/mozilla/mozilla.status ======================================= --- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Mon Aug 3 06:17:34 2009 +++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Aug 5 04:08:24 2009 @@ -54,41 +54,47 @@ Register scratch) { Label fast; - // Compute the page address from the heap object pointer, leave it - // in 'object'. + // Compute the page start address from the heap object pointer, and reuse + // the 'object' register for it. masm->and_(object, ~Page::kPageAlignmentMask); - - // Compute the bit addr in the remembered set, leave it in "addr". - masm->sub(addr, Operand(object)); + Register page_start = object; + + // Compute the bit addr in the remembered set/index of the pointer in the + // page. Reuse 'addr' as pointer_offset. + masm->sub(addr, Operand(page_start)); masm->shr(addr, kObjectAlignmentBits); + Register pointer_offset = addr; // If the bit offset lies beyond the normal remembered set range, it is in // the extra remembered set area of a large object. - masm->cmp(addr, Page::kPageSize / kPointerSize); + masm->cmp(pointer_offset, Page::kPageSize / kPointerSize); masm->j(less, &fast); - // Adjust 'addr' to be relative to the start of the extra remembered set - // 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. - 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)); - + // Adjust 'page_start' so that addressing using 'pointer_offset' hits the + // extra remembered set after the large object. + + // Find the length of the large object (FixedArray). + masm->mov(scratch, Operand(page_start, Page::kObjectStartOffset + + FixedArray::kLengthOffset)); + Register array_length = scratch; + + // Extra remembered set starts right after the large object (a FixedArray), at + // page_start + kObjectStartOffset + objectSize + // where objectSize is FixedArray::kHeaderSize + kPointerSize * array_length. + // Add the delta between the end of the normal RSet and the start of the + // extra RSet to 'object', so that addressing the bit using 'pointer_offset' + // hits the extra RSet words. + masm->lea(page_start, + Operand(page_start, array_length, 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(page_start, Page::kRSetOffset), pointer_offset); } ======================================= --- /branches/bleeding_edge/src/spaces-inl.h Mon Aug 3 04:05:26 2009 +++ /branches/bleeding_edge/src/spaces-inl.h Wed Aug 5 04:08:24 2009 @@ -127,20 +127,19 @@ if (rset_address >= page->RSetEnd()) { // We have a large object page, and the remembered set address is actually - // past the end of the object. The address of the remembered set in this - // case is the extra remembered set start address at the address of the - // end of the object: + // past the end of the object. + + // The first part of the remembered set is still located at the start of + // the page, but anything after kRSetEndOffset must be relocated to after + // the large object, i.e. after // (page->ObjectAreaStart() + object size) - // plus the offset of the computed remembered set address from the start - // of the object: - // (rset_address - page->ObjectAreaStart()). - // Ie, we can just add the object size. - // In the X64 architecture, the remembered set ends before the object start, - // so we need to add an additional offset, from rset end to object start + // We do that by adding the difference between the normal RSet's end and + // the object's end. ASSERT(HeapObject::FromAddress(address)->IsFixedArray()); - rset_address += kObjectStartOffset - kRSetEndOffset + + int fixedarray_length = FixedArray::SizeFor(Memory::int_at(page->ObjectAreaStart() + Array::kLengthOffset)); + rset_address += kObjectStartOffset - kRSetEndOffset + fixedarray_length; } return rset_address; } ======================================= --- /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Mon Aug 3 04:05:26 2009 +++ /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Wed Aug 5 04:08:24 2009 @@ -53,43 +53,50 @@ Register scratch) { Label fast; - // Compute the page address from the heap object pointer, leave it - // in 'object'. + // Compute the page start address from the heap object pointer, and reuse + // the 'object' register for it. ASSERT(is_int32(~Page::kPageAlignmentMask)); masm->and_(object, Immediate(static_cast<int32_t>(~Page::kPageAlignmentMask))); - - // Compute the bit addr in the remembered set, leave it in "addr". - masm->subq(addr, object); + Register page_start = object; + + // Compute the bit addr in the remembered set/index of the pointer in the + // page. Reuse 'addr' as pointer_offset. + masm->subq(addr, page_start); masm->shr(addr, Immediate(kPointerSizeLog2)); + Register pointer_offset = addr; // If the bit offset lies beyond the normal remembered set range, it is in // the extra remembered set area of a large object. - masm->cmpq(addr, Immediate(Page::kPageSize / kPointerSize)); + masm->cmpq(pointer_offset, Immediate(Page::kPageSize / kPointerSize)); masm->j(less, &fast); - // Adjust 'addr' to be relative to the start of the extra remembered set - // and the page address in 'object' to be the address of the extra - // remembered set. - masm->subq(addr, Immediate(Page::kPageSize / kPointerSize)); + // Adjust 'page_start' so that addressing using 'pointer_offset' hits the + // extra remembered set after the large object. + // Load the array length into 'scratch'. masm->movl(scratch, - Operand(object, + Operand(page_start, Page::kObjectStartOffset + FixedArray::kLengthOffset)); - // Extra remembered set starts right after FixedArray. - // Add the page header, array header, and array body size - // (length * pointer size) to the page address to find the extra remembered - // set start. - masm->lea(object, - Operand(object, scratch, times_pointer_size, - Page::kObjectStartOffset + FixedArray::kHeaderSize)); + Register array_length = scratch; + + // Extra remembered set starts right after the large object (a FixedArray), at + // page_start + kObjectStartOffset + objectSize + // where objectSize is FixedArray::kHeaderSize + kPointerSize * array_length. + // Add the delta between the end of the normal RSet and the start of the + // extra RSet to 'page_start', so that addressing the bit using + // 'pointer_offset' hits the extra RSet words. + masm->lea(page_start, + Operand(page_start, array_length, 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, Page::kRSetOffset), addr); + masm->bts(Operand(page_start, Page::kRSetOffset), pointer_offset); } @@ -181,7 +188,7 @@ } else { // array access: calculate the destination address in the same manner as // KeyedStoreIC::GenerateGeneric. Multiply a smi by 4 to get an offset - // into an array of words. + // into an array of pointers. lea(dst, Operand(object, dst, times_half_pointer_size, FixedArray::kHeaderSize - kHeapObjectTag)); } ======================================= --- /branches/bleeding_edge/test/mozilla/mozilla.status Tue Aug 4 07:18:03 2009 +++ /branches/bleeding_edge/test/mozilla/mozilla.status Wed Aug 5 04:08:24 2009 @@ -816,15 +816,3 @@ js1_5/extensions/regress-336410-1: CRASH js1_5/Function/regress-338001: FAIL || CRASH js1_5/extensions/regress-371636: CRASH -# The following failures were added when remembered sets were enabled. -js1_5/GC/regress-203278-2: FAIL || PASS || CRASH -js1_5/GC/regress-203278-3: FAIL || PASS -js1_5/Regress/regress-280769-3: FAIL || PASS -js1_5/Regress/regress-280769-4: CRASH || TIMEOUT -js1_5/Regress/regress-290575: CRASH -js1_5/extensions/regress-365692: FAIL || PASS -js1_5/Regress/regress-366601: FAIL -js1_5/Regress/regress-367561-03: CRASH -js1_5/Regress/regress-367561-01: CRASH || PASS -ecma/Expressions/11.7.2: CRASH - --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
