Does this mean that we can have the object be tagged and the index have the heap tag subtracted, i.e. ends with two 1 bits? I suggest that we put a check here (guarded by --debug-code) that checks that the object is a tagged pointer and the index is a valid index ending with two 0 bits. If we for performance reasons have places where this does not hold use write barrier code where this is explicit (and checked), e.g. WriteBarrierWithSkewedIndex or WriteBarrierWithNonTaggedObject.
/Søren On Thu, Apr 15, 2010 at 16:18, <[email protected]> wrote: > Reviewers: William Hesse, > > Description: > Fix constant offset check for inlined write barrier to work in cases when > offset > is given from tagged object pointer. > > Please review this at http://codereview.chromium.org/1646008/show > > SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ > > Affected files: > M src/ia32/macro-assembler-ia32.cc > M src/x64/macro-assembler-x64.cc > > > Index: src/ia32/macro-assembler-ia32.cc > =================================================================== > --- src/ia32/macro-assembler-ia32.cc (revision 4428) > +++ src/ia32/macro-assembler-ia32.cc (working copy) > @@ -143,7 +143,18 @@ > > InNewSpace(object, value, equal, &done); > > - if ((offset > 0) && (offset < Page::kMaxHeapObjectSize)) { > + // We are storing pointer to an object so either offset or > + // offset + kHeapObjectTag should be pointer size aligned > + // depending on whether register object contains untagged > + // or tagged pointer to heap object. > + ASSERT(IsAligned(offset, kPointerSize) || > + IsAligned(offset + kHeapObjectTag, kPointerSize)); > + > + // We are using fast write barrier for small offsets (rset bits > corresponding > + // to them are at the beggining of the page). We are comparing against > + // Page::kMaxHeapObjectSize - kHeapObjectTag to catch cases when pointer > in > + // object register is tagged and offset was adjusted to accomodate that. > + if ((offset > 0) && (offset < Page::kMaxHeapObjectSize - > kHeapObjectTag)) { > // Compute the bit offset in the remembered set, leave it in 'value'. > lea(value, Operand(object, offset)); > and_(value, Page::kPageAlignmentMask); > Index: src/x64/macro-assembler-x64.cc > =================================================================== > --- src/x64/macro-assembler-x64.cc (revision 4428) > +++ src/x64/macro-assembler-x64.cc (working copy) > @@ -226,7 +226,18 @@ > cmpq(scratch, kScratchRegister); > j(equal, &done); > > - if ((offset > 0) && (offset < Page::kMaxHeapObjectSize)) { > + // We are storing pointer to an object so either offset or > + // offset + kHeapObjectTag should be pointer size aligned > + // depending on whether register object contains untagged > + // or tagged pointer to heap object. > + ASSERT(IsAligned(offset, kPointerSize) || > + IsAligned(offset + kHeapObjectTag, kPointerSize)); > + > + // We are using fast write barrier for small offsets (rset bits > corresponding > + // to them are at the beggining of the page). We are comparing against > + // Page::kMaxHeapObjectSize - kHeapObjectTag to catch cases when pointer > in > + // object register is tagged and offset was adjusted to accomodate that. > + if ((offset > 0) && (offset < Page::kMaxHeapObjectSize - > kHeapObjectTag)) { > // Compute the bit offset in the remembered set, leave it in 'value'. > lea(scratch, Operand(object, offset)); > ASSERT(is_int32(Page::kPageAlignmentMask)); > > > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > > To unsubscribe, reply using "remove me" as the subject. > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
