X64: Implement fast charCodeAt.

X64: Implement fast charCodeAt.

[Lasse Reichstein, 2009-08-07 07:52:08]

Review of direct translation from ia32.

[William Hesse, 2009-08-07 10:27:15]

LGTM, with comments.

http://codereview.chromium.org/164135/diff/1/4
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/164135/diff/1/4#newcode3492
Line 3492: Immediate(static_cast<uint32_t>(kSmiTagMask | 0x80000000U)));
shouldn't this be a static cast to int32_t.  I think the implicit cast from
uint32_t constant to int32_t which is negative causes the warning.

http://codereview.chromium.org/164135/diff/1/4#newcode3516
Line 3516: __ shrl(temp.reg());  // The shift amount in ecx is implicit operand.
cl (low byte of rcx), not ecx.
Is this a shr or sar?  Is the high bit really 0?

http://codereview.chromium.org/164135/diff/1/4#newcode3519
Line 3519: __ j(greater_equal, &slow_case);
greater_equal or below_equal?  Are either of these really negative?

http://codereview.chromium.org/164135/diff/1/4#newcode3534
Line 3534: __ movzxwl(temp.reg(), FieldOperand(object.reg(),
Add the new operations to the disassembler.

[Lasse Reichstein, 2009-08-07 10:44:27]

http://codereview.chromium.org/164135/diff/1/4
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/164135/diff/1/4#newcode3492
Line 3492: Immediate(static_cast<uint32_t>(kSmiTagMask | 0x80000000U)));
Done.

http://codereview.chromium.org/164135/diff/1/4#newcode3516
Line 3516: __ shrl(temp.reg());  // The shift amount in ecx is implicit operand.
Not sure whether high bit can be 1 (would be waste of a bit if it can't), but if
it can, this should be shr. It's a String's length, shifted left, so it should
be positive.

http://codereview.chromium.org/164135/diff/1/4#newcode3519
Line 3519: __ j(greater_equal, &slow_case);
The string length is definitly positive, the index was tested to be non-negative
above (before un-smi-tagging). I.e., it doesn't matter.
I'll change it to above_equal to suggest the we are dealing with positive
numbers only.

http://codereview.chromium.org/164135/diff/1/4#newcode3534
Line 3534: __ movzxwl(temp.reg(), FieldOperand(object.reg(),
Already there. It's just a another operand size for movzxb and movzxw.

http://codereview.chromium.org/164135/diff/1/4#newcode3534
Line 3534: __ movzxwl(temp.reg(), FieldOperand(object.reg(),
Already there. It's just a another operand size for movzxb and movzxw.