LGTM

http://codereview.chromium.org/209048/diff/1/2
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/209048/diff/1/2#newcode480
Line 480: int holes,
Rename "holes" to "initial_capacity".

http://codereview.chromium.org/209048/diff/1/2#newcode494
Line 494: __ AllocateObjectInNewSpace(size,
You are not really allocating an object in NewSpace, but just a patch of
memory for two objects.
Should the allocate function be renamed to AllocateInNewSpace(...)?

http://codereview.chromium.org/209048/diff/1/2#newcode553
Line 553: __ Move(Operand(scratch1, 0), Factory::the_hole_value());
You should use the scratch3 register for the "hole" value here as well.
The way it is done will reload the value into kScratchRegister, using a
a 64-bit immediate, on every round (adding 10+ bytes to the loop).

http://codereview.chromium.org/209048/diff/1/2#newcode674
Line 674: __ j(below, &loop);
Why not put the test before the loop, in "while"-style?

http://codereview.chromium.org/209048/diff/1/2#newcode720
Line 720: Condition cond = __ CheckNotPositiveSmi(rdx);
rename "cond" to not_positive_smi. Makes the jump read much better.
(Or use JumpIfNotPositiveSmi(rdx, call_generic_code), if it exists).

http://codereview.chromium.org/209048/diff/1/2#newcode726
Line 726: __ cmpq(rdx, Immediate(JSObject::kInitialMaxFastElementArray
<< kSmiTagSize));
Don't create smis like this (if the immediate is really a smi, and not
just tagged as a smi).
Use:
   __ JumpIfSmiEqualsConstant(rdx, JSObject::kInitialMxFastElementArray,
call_generic_code)

http://codereview.chromium.org/209048/diff/1/2#newcode826
Line 826: __ testq(rbx, Immediate(kSmiTagMask));
Use
  Condition is_smi = CheckSmi(rbx);
... as long as NULL is a smi. Check that with ASSERT_EQ(0, kSmiTag);

http://codereview.chromium.org/209048/diff/1/2#newcode862
Line 862: __ testq(rbx, Immediate(kSmiTagMask));
Use CheckSmi as above.

http://codereview.chromium.org/209048

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

Reply via email to