Please review the added andl instruction to the assembler and the CheckSmiGreaterEqualsConstant in the macro assembler.
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, On 2009/09/22 11:29:34, Lasse Reichstein wrote: > Rename "holes" to "initial_capacity". Done. Changed in IA32 and ARM versions as well. http://codereview.chromium.org/209048/diff/1/2#newcode494 Line 494: __ AllocateObjectInNewSpace(size, On 2009/09/22 11:29:34, Lasse Reichstein wrote: > 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(...)? Good point. I will rename this in a separate changelist. http://codereview.chromium.org/209048/diff/1/2#newcode553 Line 553: __ Move(Operand(scratch1, 0), Factory::the_hole_value()); On 2009/09/22 11:29:34, Lasse Reichstein wrote: > 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). Done. Artifacts from the IA32 version. http://codereview.chromium.org/209048/diff/1/2#newcode674 Line 674: __ j(below, &loop); On 2009/09/22 11:29:34, Lasse Reichstein wrote: > Why not put the test before the loop, in "while"-style? This is the way a number of other loops in this file are written, so it's mainly derived from copy/paste. http://codereview.chromium.org/209048/diff/1/2#newcode720 Line 720: Condition cond = __ CheckNotPositiveSmi(rdx); On 2009/09/22 11:29:34, Lasse Reichstein wrote: > rename "cond" to not_positive_smi. Makes the jump read much better. > (Or use JumpIfNotPositiveSmi(rdx, call_generic_code), if it exists). Done. http://codereview.chromium.org/209048/diff/1/2#newcode726 Line 726: __ cmpq(rdx, Immediate(JSObject::kInitialMaxFastElementArray << kSmiTagSize)); On 2009/09/22 11:29:34, Lasse Reichstein wrote: > 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) Added JumpIfSmiGreaterEqualsConstant to macro assembler to cover this. http://codereview.chromium.org/209048/diff/1/2#newcode826 Line 826: __ testq(rbx, Immediate(kSmiTagMask)); On 2009/09/22 11:29:34, Lasse Reichstein wrote: > Use > Condition is_smi = CheckSmi(rbx); > ... as long as NULL is a smi. Check that with ASSERT_EQ(0, kSmiTag); Done. http://codereview.chromium.org/209048/diff/1/2#newcode862 Line 862: __ testq(rbx, Immediate(kSmiTagMask)); On 2009/09/22 11:29:34, Lasse Reichstein wrote: > Use CheckSmi as above. Done. http://codereview.chromium.org/209048 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
