http://codereview.chromium.org/3341012/diff/14001/15002
File src/arm/code-stubs-arm.cc (left):

http://codereview.chromium.org/3341012/diff/14001/15002#oldcode4142
src/arm/code-stubs-arm.cc:4142: static const int kFromOffset = 1 *
kPointerSize;
Please leave this constant there because it serves to document the
layout on the stack.

http://codereview.chromium.org/3341012/diff/14001/15002
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/3341012/diff/14001/15002#newcode4149
src/arm/code-stubs-arm.cc:4149: __ Ldrd(to, from, MemOperand(sp,
kToOffset));
Here you should statically assert that the FromOffset is the ToOffset +
4.  That way if someone tries to rearrange things they will get an
error.

http://codereview.chromium.org/3341012/diff/14001/15002#newcode4234
src/arm/code-stubs-arm.cc:4234: masm, r3, r4, r1, r5, to, from, r9,
&make_two_character_string);
To and from are just being used as scratch registers here.  Therefore it
is misleading to use their names.  I would rather just use r6 and r7 on
this line.

http://codereview.chromium.org/3341012/diff/14001/15002#newcode4267
src/arm/code-stubs-arm.cc:4267:
StringHelper::GenerateCopyCharactersLong(masm, r1, r5, r2, r3, r4, to,
from,
Likewise here.

http://codereview.chromium.org/3341012/diff/14001/15002#newcode4268
src/arm/code-stubs-arm.cc:4268: r9, COPY_ASCII | DEST_ALWAYS_ALIGNED);
Indentation is wrong here now.

http://codereview.chromium.org/3341012/diff/14001/15002#newcode4276
src/arm/code-stubs-arm.cc:4276: // Check for flat two byte string.
Since the value in the 'from' register is used lower down it is still
live.  It is problematic to remove the comment here and similar ones
above since they documented that fact.

http://codereview.chromium.org/3341012/diff/14001/15002#newcode4297
src/arm/code-stubs-arm.cc:4297:
StringHelper::GenerateCopyCharactersLong(masm, r1, r5, r2, r3, r4, to,
from,
Again, these are just scratch registers.  The names 'from' and 'to' are
misleading.

http://codereview.chromium.org/3341012/diff/14001/15002#newcode4374
src/arm/code-stubs-arm.cc:4374: __ Ldrd(r0 , r1, MemOperand(sp));  //
Load right in r0, left in r1
Full stop/period at the end of comments.

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

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

Reply via email to