http://codereview.chromium.org/3329019/diff/15001/16001
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/3329019/diff/15001/16001#newcode693
src/arm/builtins-arm.cc:693: }  // Otherwise r7 already holds undefined.
might be worth checking if FLAG_debug_code is enabled.

http://codereview.chromium.org/3329019/diff/15001/16006
File src/ia32/assembler-ia32.h (right):

http://codereview.chromium.org/3329019/diff/15001/16006#newcode598
src/ia32/assembler-ia32.h:598: void dec_b(const Operand& dst);
apparently still no changes to disassembler

http://codereview.chromium.org/3329019/diff/15001/16007
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/3329019/diff/15001/16007#newcode191
src/ia32/builtins-ia32.cc:191: // To allow for truncation.
nit: do you want to unify with other platforms?  e.g. there is no such
comment in ARM port.

http://codereview.chromium.org/3329019/diff/15001/16007#newcode193
src/ia32/builtins-ia32.cc:193: __ mov(edx,
Factory::one_pointer_filler_map());
we should never come here for API functions, correct?  could we add
Assert here?

http://codereview.chromium.org/3329019/diff/15001/16008
File src/objects-inl.h (right):

http://codereview.chromium.org/3329019/diff/15001/16008#newcode2575
src/objects-inl.h:2575: kInobjectSlackTrackingMapOffset)
it is somewhat weird to see initial_map accessor with
kInobjectSlackTrackingMapOffset.  Could the names be adjusted?

http://codereview.chromium.org/3329019/diff/15001/16009
File src/objects.cc (right):

http://codereview.chromium.org/3329019/diff/15001/16009#newcode5446
src/objects.cc:5446: void SharedFunctionInfo::ReleaseInitialMap() {
apparently this CL is missing GC-part of the changes

http://codereview.chromium.org/3329019/diff/15001/16009#newcode5503
src/objects.cc:5503: void
SharedFunctionInfo::CompleteInobjectSlackTracking() {
does that mean that after CompleteInobjectSlackTracking is over, another
context might initiate the process once again?

If yes, do we want it?

http://codereview.chromium.org/3329019/diff/15001/16010
File src/objects.h (right):

http://codereview.chromium.org/3329019/diff/15001/16010#newcode3243
src/objects.h:3243: typedef int (*TraverseCallback)(Map* map, void*
data);
apparently you don't need return type int for the callback--it's never
checked

http://codereview.chromium.org/3329019/diff/15001/16010#newcode3459
src/objects.h:3459: // after the constructor). There is no guarantee
ther the extra space
ther -> that?

http://codereview.chromium.org/3329019/diff/15001/16010#newcode3538
src/objects.h:3538: // May to go back from true to false if no
constructed objects survived GC.
May to go -> may go

Maybe rename to HasLiveObjects()?

http://codereview.chromium.org/3329019/diff/15001/16013
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/3329019/diff/15001/16013#newcode1005
src/x64/builtins-x64.cc:1005: // To allow for truncation
nit: missing dot.

http://codereview.chromium.org/3329019/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to