All done. Please take yet another look.
Thanks, Vitaly http://codereview.chromium.org/3144002/diff/1/13 File src/objects-inl.h (right): http://codereview.chromium.org/3144002/diff/1/13#newcode1401 src/objects-inl.h:1401: ASSERT(map_word().ToMap() != Heap::raw_unchecked_fixed_cow_array_map()); On 2010/08/13 16:15:08, Vyacheslav Egorov wrote:
On 2010/08/13 14:14:01, Vitaly wrote: > On 2010/08/11 10:42:31, Mads Ager wrote: > > Just use 'map()' in all of these asserts? > > > > Are these used during GC so that you need to use > > raw_unchecked_fixed_cow_array_map or could you just use fixed_cow_array_map()? > > Changed map_word().ToMap() to map(). Some of these are used during
GC (our
> descriptor dance) so I used "unchecked" in all of them for
consistency.
>
If some of them are used during GC then they should take into account
that
object can be marked. For marked object assertion
ASSERT(map() != Heap::raw_unchecked_fixed_cow_array_map());
will never fail even if object actually have COW array map.
During GC you have to clear mark and overflow bits from map word to
get a real
map pointer.
Right, but given that during GC the value of the map word can be almost anything, we can't write a reliable map check here. I simply was trying to make it not crash during GC. We discussed the issue with Slava. He had an idea to have a generic safe way to get the map during GC (using a global function pointer updated by various GC phases). We agreed that this is an overkill here and decided to add unchecked setters for the GC to use. http://codereview.chromium.org/3144002/diff/4002/13004 File src/arm/ic-arm.cc (right): http://codereview.chromium.org/3144002/diff/4002/13004#newcode1143 src/arm/ic-arm.cc:1143: __ ldr(r4, FieldMemOperand(r1, JSObject::kElementsOffset)); On 2010/08/13 16:50:47, antonm wrote:
Do we need to load elements, shouldn't they be in r2 already
(otherwise comment
for GenerateFastArrayLoad should be corrected).
Because of the "has fast elements" bit check we now can get to this code before even trying to load the elements. http://codereview.chromium.org/3144002/diff/4002/13029 File src/x64/ic-x64.cc (right): http://codereview.chromium.org/3144002/diff/4002/13029#newcode640 src/x64/ic-x64.cc:640: __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset)); On 2010/08/13 16:50:47, antonm wrote:
again looks like rcx should already hold elements.
See above. http://codereview.chromium.org/3144002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
