LGTM

http://codereview.chromium.org/2731007/diff/23001/24001
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/2731007/diff/23001/24001#newcode5093
src/arm/codegen-arm.cc:5093: frame_->CallStub(&stub, 1);
You could do
SpillAllButCopyTOSToR0()
and then have the calling convention for the stub be that the argument
is passed in R0 as well as on the stack.  The stub call will spill all
anyway, but this way you don't have to load r0 at the start of the stub.

http://codereview.chromium.org/2731007/diff/23001/24001#newcode8269
src/arm/codegen-arm.cc:8269: Factory::heap_number_map(),
It would be nice if we had an overloaded version of CheckMap that took a
root array index.

http://codereview.chromium.org/2731007/diff/23001/24001#newcode8285
src/arm/codegen-arm.cc:8285: __ and_(r3, r3,
Operand(TranscendentalCache::kCacheSize - 1));
This is anding with 511, which doesn't fit in the immediate field, so
there's an implicit pc-relative load into ip.  We could use the ubfx
instruction on ARM7.

http://codereview.chromium.org/2731007/diff/23001/24001#newcode8296
src/arm/codegen-arm.cc:8296: __ tst(r0, Operand(r0));
Prefer cmp with immediate 0 here, since it doesn't do a partial flags
update.

http://codereview.chromium.org/2731007/diff/23001/24001#newcode8318
src/arm/codegen-arm.cc:8318: __ ldr(r4, MemOperand(r0, 0));
Load r4, r5 and r6 with ldm?

http://codereview.chromium.org/2731007/diff/23001/24003
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/2731007/diff/23001/24003#newcode10293
src/ia32/codegen-ia32.cc:10293: __ shr(eax, 16);
Oops!  Nice catch!

http://codereview.chromium.org/2731007/show

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

Reply via email to