ARM port LGTM if you address the comments.

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/full-codegen-arm.cc#newcode1151
src/arm/full-codegen-arm.cc:1151: __ Push(r1, r2, r0);
Please try to keep the registers that are being pushed in descending
order.

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/lithium-arm.h
File src/arm/lithium-arm.h (right):

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/lithium-arm.h#newcode1013
src/arm/lithium-arm.h:1013: "map-enum-length")
nit: fits on one line

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/lithium-arm.h#newcode1014
src/arm/lithium-arm.h:1014: DECLARE_HYDROGEN_ACCESSOR(MapEnumLength)
I don't see why you need this (not on ia32 either).

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/macro-assembler-arm.cc#newcode3748
src/arm/macro-assembler-arm.cc:3748: cmp(r3,
Operand(Smi::FromInt(Map::kInvalidEnumCache)));
On ia32, this compares against Smi::FromInt(0). Is the ARM version
different on purpose?

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/macro-assembler-arm.cc#newcode3753
src/arm/macro-assembler-arm.cc:3753: // Check that there are no
elements. Register rcx contains the current JS
s/rcx/r2/, I guess

https://chromiumcodereview.appspot.com/10824079/diff/17008/src/arm/macro-assembler-arm.cc#newcode3756
src/arm/macro-assembler-arm.cc:3756: cmp(r2, r6);
s/r6/empty_fixed_array_value/ please

https://chromiumcodereview.appspot.com/10824079/

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

Reply via email to