Feedback addressed, landing
https://chromiumcodereview.appspot.com/10544005/diff/1/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): https://chromiumcodereview.appspot.com/10544005/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode3575 src/ia32/lithium-codegen-ia32.cc:3575: __ cmp(FieldOperand(object_reg, HeapObject::kMapOffset), from_map); On 2012/06/06 09:41:17, Michael Starzinger wrote:
Can we move this "__ cmp" down so that it is right above the "__ j"?
That would
improve readability.
Done. https://chromiumcodereview.appspot.com/10544005/diff/1/src/ia32/macro-assembler-ia32.cc File src/ia32/macro-assembler-ia32.cc (right): https://chromiumcodereview.appspot.com/10544005/diff/1/src/ia32/macro-assembler-ia32.cc#newcode274 src/ia32/macro-assembler-ia32.cc:274: // object's interesting flag is also set. On 2012/06/06 09:41:17, Michael Starzinger wrote:
Can we add to the comment that this relies on "maps not being
compacted" and
"maps never being in new-space". We can even assert that ...
ASSERT(!isolate()->heap()->InNewSpace(*map));
ASSERT(!isolate()->heap()->mark_compact_collector()->IsOnEvacuationCandidate(*map));
The second assert is not as useful as the first, because it only
checks whether
the map is on an evacuation candidate during compile-time, but not
during
runtime. But I would still include it.
Done. https://chromiumcodereview.appspot.com/10544005/diff/1/src/ia32/macro-assembler-ia32.cc#newcode2692 src/ia32/macro-assembler-ia32.cc:2692: Page* page = reinterpret_cast<Page*>(masked_address); On 2012/06/06 09:41:17, Michael Starzinger wrote:
Better use the following "Page* page =
Page::FromAddress(map->address());" here. Done. https://chromiumcodereview.appspot.com/10544005/diff/1/src/ia32/macro-assembler-ia32.h File src/ia32/macro-assembler-ia32.h (right): https://chromiumcodereview.appspot.com/10544005/diff/1/src/ia32/macro-assembler-ia32.h#newcode93 src/ia32/macro-assembler-ia32.h:93: void CheckPageFlag(Handle<Map> map, On 2012/06/06 09:41:17, Michael Starzinger wrote:
I am not sure if we should rename this to CheckPageFlagForMap, I'll
leave that
up to you.
Done. https://chromiumcodereview.appspot.com/10544005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
