LGTM
http://codereview.chromium.org/193125/diff/1/5 File src/builtins.cc (left): http://codereview.chromium.org/193125/diff/1/5#oldcode169 Line 169: if (args.length() == 2) return array->SetElementsLength(args[1]); Good catch. :) 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 holes larger -> holes is larger http://codereview.chromium.org/193125/diff/1/3#newcode698 Line 698: size += FixedArray::kHeaderSize + holes * kPointerSize; You could use FixedArray::SizeFor(holes) here instead. http://codereview.chromium.org/193125/diff/1/3#newcode713 Line 713: __ mov(scratch1, Factory::empty_fixed_array()); You don't need to use the scratch register here. You should be able to use __ mov(FieldOperand(...), Factory::empty_fixed_array())? 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 the the -> the http://codereview.chromium.org/193125/diff/1/3#newcode721 Line 721: __ mov(scratch3, Factory::empty_fixed_array()); No need for using the scratch register. http://codereview.chromium.org/193125/diff/1/3#newcode738 Line 738: __ mov(scratch3, Factory::fixed_array_map()); Remove? http://codereview.chromium.org/193125/diff/1/3#newcode743 Line 743: __ mov(scratch3, Factory::the_hole_value()); 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)? http://codereview.chromium.org/193125/diff/1/3#newcode795 Line 795: FixedArray::kHeaderSize + FixedArray::SizeFor(kPreallocatedArrayElements)? http://codereview.chromium.org/193125/diff/1/3#newcode847 Line 847: __ mov(scratch, Factory::fixed_array_map()); Remove? http://codereview.chromium.org/193125/diff/1/3#newcode869 Line 869: __ mov(scratch, Factory::the_hole_value()); Remove. Do you want to unfold this loop as well if it is small? http://codereview.chromium.org/193125/diff/1/3#newcode1041 Line 1041: // REstore argc and constructor before running the generic code. REstore -> Restore. http://codereview.chromium.org/193125/diff/1/3#newcode1062 Line 1062: #ifdef DEBUG if (FLAG_debug_code) { ? http://codereview.chromium.org/193125/diff/1/3#newcode1093 Line 1093: #ifdef DEBUG if (FLAG_debug_code) { ? http://codereview.chromium.org/193125 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
