LGTM but we need a test that feeds Smis into the regexp cache routines (test
equivalent).


http://codereview.chromium.org/3034060/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/3034060/diff/1/2#newcode3312
src/arm/codegen-arm.cc:3312: literal = frame_->PopToRegister(r0);
I find it confusing that the literal variable is reused here for a new
register.  Can we either call it something else or add a comment to say
that we are reusing the variable?

http://codereview.chromium.org/3034060/diff/1/2#newcode3316
src/arm/codegen-arm.cc:3316: __ str(tmp, FieldMemOperand(r0, i));
This is the same code as in FastCloneShallowArrayStub.  We should move
this sort of memcpy with fixed size into the macro assembler.  There we
could also make it more efficient.  With 2 scratch registers and ip we
could do it with 3-register ldm and stm which is more compact and on
some chips faster.

http://codereview.chromium.org/3034060/diff/1/2#newcode5347
src/arm/codegen-arm.cc:5347: Register tmp = r2;
Please use the scratch registers provided by the virtual frame.

http://codereview.chromium.org/3034060/diff/1/2#newcode5350
src/arm/codegen-arm.cc:5350: Label done;
Please add a comment to the effect that the state of the CC flags at
this label is significant.

http://codereview.chromium.org/3034060/diff/1/4
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/3034060/diff/1/4#newcode1140
src/arm/full-codegen-arm.cc:1140: __ str(r2, FieldMemOperand(r0, i));
And here

http://codereview.chromium.org/3034060/diff/1/14
File src/runtime.cc (right):

http://codereview.chromium.org/3034060/diff/1/14#newcode7600
src/runtime.cc:7600: RUNTIME_ASSERT(size <=
Heap::MaxObjectSizeInNewSpace());
size actually needs to be smaller than the minimum free space in new
space after a GC, which is less thatn the MaxObjectSizeInNewSpace.

http://codereview.chromium.org/3034060/diff/1/16
File src/string.js (right):

http://codereview.chromium.org/3034060/diff/1/16#newcode625
src/string.js:625: // Reuse lastIndex field for split limit when type is
"split".
This was wrong before, huh?

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

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

Reply via email to