http://codereview.chromium.org/193125/diff/1/3 File src/ia32/builtins-ia32.cc (right):
http://codereview.chromium.org/193125/diff/1/3#newcode677 Line 677: // register. If the parameter holes larger than zero an elements backing store On 2009/09/16 10:46:13, Mads Ager wrote: > holes larger -> holes is larger Done. http://codereview.chromium.org/193125/diff/1/3#newcode698 Line 698: size += FixedArray::kHeaderSize + holes * kPointerSize; On 2009/09/16 10:46:13, Mads Ager wrote: > You could use FixedArray::SizeFor(holes) here instead. Done. http://codereview.chromium.org/193125/diff/1/3#newcode713 Line 713: __ mov(scratch1, Factory::empty_fixed_array()); On 2009/09/16 10:46:13, Mads Ager wrote: > You don't need to use the scratch register here. You should be able to use __ > mov(FieldOperand(...), Factory::empty_fixed_array())? Done - I guess I was being ARM'ish. http://codereview.chromium.org/193125/diff/1/3#newcode718 Line 718: // If no storage is requested for the the elements array just set the empty On 2009/09/16 10:46:13, Mads Ager wrote: > the the -> the Done. http://codereview.chromium.org/193125/diff/1/3#newcode721 Line 721: __ mov(scratch3, Factory::empty_fixed_array()); On 2009/09/16 10:46:13, Mads Ager wrote: > No need for using the scratch register. Done. http://codereview.chromium.org/193125/diff/1/3#newcode738 Line 738: __ mov(scratch3, Factory::fixed_array_map()); On 2009/09/16 10:46:13, Mads Ager wrote: > Remove? Done. http://codereview.chromium.org/193125/diff/1/3#newcode743 Line 743: __ mov(scratch3, Factory::the_hole_value()); On 2009/09/16 10:46:13, Mads Ager wrote: > Push this into only the unfolded loop to avoid the extra reloc info? Use it > directly in the loop that is not unfolded (since there will be only one > occurrence)? Done. http://codereview.chromium.org/193125/diff/1/3#newcode795 Line 795: FixedArray::kHeaderSize + On 2009/09/16 10:46:13, Mads Ager wrote: > FixedArray::SizeFor(kPreallocatedArrayElements)? Done. http://codereview.chromium.org/193125/diff/1/3#newcode847 Line 847: __ mov(scratch, Factory::fixed_array_map()); On 2009/09/16 10:46:13, Mads Ager wrote: > Remove? Done. http://codereview.chromium.org/193125/diff/1/3#newcode869 Line 869: __ mov(scratch, Factory::the_hole_value()); On 2009/09/16 10:46:13, Mads Ager wrote: > Remove. > Do you want to unfold this loop as well if it is small? Can't do that as the size is dynamic and in a register. http://codereview.chromium.org/193125/diff/1/3#newcode940 Line 940: __ test(Operand(esp, (push_count + 1) * kPointerSize), Immediate(0xc0000000)); On 2009/09/16 08:03:01, Lasse Reichstein wrote: > You can combine the test for a non-negative smi into a single test: > test(Operand(esp, (push_count + 1) * kPointerSize), > Immediate(0x80000001)); > j(not_zero, &prepeare_generic_code_call); Done. Used kIntprtSignBit | kSmiTagMask instead of 0x80000001. The 0xc0000000 was also wrong as the value was a smi. http://codereview.chromium.org/193125/diff/1/3#newcode1041 Line 1041: // REstore argc and constructor before running the generic code. On 2009/09/16 10:46:13, Mads Ager wrote: > REstore -> Restore. Done. http://codereview.chromium.org/193125/diff/1/3#newcode1062 Line 1062: #ifdef DEBUG On 2009/09/16 10:46:13, Mads Ager wrote: > if (FLAG_debug_code) { > ? Done. Changed Check to Assert. We have much inconsistency in the code of whether to use Assert or Check when checking for FLAG_debug_code. http://codereview.chromium.org/193125/diff/1/3#newcode1093 Line 1093: #ifdef DEBUG On 2009/09/16 10:46:13, Mads Ager wrote: > if (FLAG_debug_code) { > ? Done. http://codereview.chromium.org/193125 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
