LGTM (if comments are addressed).
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); Can we move this "__ cmp" down so that it is right above the "__ j"? That would improve readability. 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. 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. 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); Better use the following "Page* page = Page::FromAddress(map->address());" here. 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, I am not sure if we should rename this to CheckPageFlagForMap, I'll leave that up to you. https://chromiumcodereview.appspot.com/10544005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
