Port of string plus smi optimization from ia32 to x64 and ARM.

Port of string plus smi optimization from ia32 to x64 and ARM.

Committed: http://code.google.com/p/v8/source/detail?r=4138

[Vyacheslav Egorov (Chromium), 2010-03-08 11:43:19]

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

http://codereview.chromium.org/668254/diff/1/2#newcode8912
src/x64/codegen-x64.cc:8912: __ movq(Operand(rsp, rax_stack_offset), rbx);
As you can see I replaced frame creation and CallStub with TailCallStub here.
But it required some strange code (see r*_stack_offset variables) and I am not
sure whether it's worth it or not.

[Søren Thygesen Gjesse, 2010-03-08 12:26:45]

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

http://codereview.chromium.org/668254/diff/1/2#newcode7210
src/x64/codegen-x64.cc:7210: ASSERT(kSmiTag == 0);
I think you can use JumpIfNotSmi from the x64 macro assembler here.

http://codereview.chromium.org/668254/diff/1/2#newcode8912
src/x64/codegen-x64.cc:8912: __ movq(Operand(rsp, rax_stack_offset), rbx);
On 2010/03/08 11:43:19, Vyacheslav Egorov wrote:
> As you can see I replaced frame creation and CallStub with TailCallStub here.
> But it required some strange code (see r*_stack_offset variables) and I am not
> sure whether it's worth it or not. 

The best way to check whether it makes a difference is to measure the
difference.

[Søren Thygesen Gjesse, 2010-03-08 12:36:09]

Some additional comments.

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

http://codereview.chromium.org/668254/diff/1/2#newcode8861
src/x64/codegen-x64.cc:8861: int rax_stack_offset, rdx_stack_offset;
I am not sure that finding the rax stack position is required. You will always
have to replace the second argument which is at the top of the stack.

http://codereview.chromium.org/668254/diff/1/2#newcode8868
src/x64/codegen-x64.cc:8868: rax_stack_offset = 2*kPointerSize;
If you need this, please add spaces around *.

http://codereview.chromium.org/668254/diff/1/2#newcode8906
src/x64/codegen-x64.cc:8906: // string for the smi in the number string cache.
Is this correct? If HasArgsReversed() is true then the second argument is in rdx
not rax.

[Vyacheslav Egorov (Chromium), 2010-03-08 13:21:06]

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

http://codereview.chromium.org/668254/diff/1/2#newcode7210
src/x64/codegen-x64.cc:7210: ASSERT(kSmiTag == 0);
On 2010/03/08 12:26:45, Søren Gjesse wrote:
> I think you can use JumpIfNotSmi from the x64 macro assembler here.

Done.

http://codereview.chromium.org/668254/diff/1/2#newcode8906
src/x64/codegen-x64.cc:8906: // string for the smi in the number string cache.
On 2010/03/08 12:36:10, Søren Gjesse wrote:
> Is this correct? If HasArgsReversed() is true then the second argument is in
rdx
> not rax.

Indeed this comment (just like others in this case) is incorrect. 

I introduced lhs/rhs register aliases in attempt to make intent clean and fix
contradiction between code and comments.

[Søren Thygesen Gjesse, 2010-03-08 13:36:58]

LGTM

Think there might be an error in the ia32 version assuming edx/eax for
left/right operands for +.