Port optimization of calls to GenericBinaryStub to x64

Port optimization of calls to GenericBinaryStub to x64.

See description of the change in the ia32 changelist at http://codereview.chromium.org/246075.

Minor changes to the ia32 version using variables for the registers to pass parameters in (edx and eax) to make the parameter set up code easier to read.

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

[Kevin Millikin (Chromium), 2009-10-26 11:23:35]

LGTM.  Some small comments that apply to both platforms.

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

http://codereview.chromium.org/335005/diff/1/2#newcode6541
Line 6541: __ mov(left_arg, left);
I like the change with the symbolic names.  Maybe this site deserves a comment
that the order of moves is important to avoid destroying right_arg/left?

http://codereview.chromium.org/335005/diff/1/2#newcode6549
Line 6549: __ mov(right_arg, right);
Order of moves also important here.

http://codereview.chromium.org/335005/diff/1/2#newcode6555
Line 6555: __ mov(left_arg, left);
Here, maybe a comment about what we know: neither left nor right is left_arg or
right_arg, and so the order of moves is not important.

http://codereview.chromium.org/335005/diff/1/2#newcode6614
Line 6614: bool is_commutative = (op_ == (Token::ADD) || (op_ == Token::MUL));
IsOperationCommutative()?

http://codereview.chromium.org/335005/diff/1/2#newcode6618
Line 6618: __ mov(right_arg, Immediate(left));
Do you want SetArgsReversed here?

http://codereview.chromium.org/335005/diff/1/5
File src/x64/codegen-x64.h (right):

http://codereview.chromium.org/335005/diff/1/5#newcode638
Line 638: // Flag that indicates whether how to generate code for the stub.
Extra 'whether'.

[Søren Thygesen Gjesse, 2009-10-26 14:40:17]

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

http://codereview.chromium.org/335005/diff/1/2#newcode6541
Line 6541: __ mov(left_arg, left);
On 2009/10/26 11:23:35, Kevin Millikin wrote:
> I like the change with the symbolic names.  Maybe this site deserves a comment
> that the order of moves is important to avoid destroying right_arg/left?

Done. Added comment in X64 version as well.

http://codereview.chromium.org/335005/diff/1/2#newcode6549
Line 6549: __ mov(right_arg, right);
On 2009/10/26 11:23:35, Kevin Millikin wrote:
> Order of moves also important here.

Done. Added comment in X64 version as well.

http://codereview.chromium.org/335005/diff/1/2#newcode6555
Line 6555: __ mov(left_arg, left);
On 2009/10/26 11:23:35, Kevin Millikin wrote:
> Here, maybe a comment about what we know: neither left nor right is left_arg
or
> right_arg, and so the order of moves is not important.

Done. Added comment in X64 version as well.

http://codereview.chromium.org/335005/diff/1/2#newcode6614
Line 6614: bool is_commutative = (op_ == (Token::ADD) || (op_ == Token::MUL));
On 2009/10/26 11:23:35, Kevin Millikin wrote:
> IsOperationCommutative()?

Done. Fixed in x64 version as well.

http://codereview.chromium.org/335005/diff/1/2#newcode6618
Line 6618: __ mov(right_arg, Immediate(left));
On 2009/10/26 11:23:35, Kevin Millikin wrote:
> Do you want SetArgsReversed here?

Thats right. This is a bug in the original version. I will try to find a test
which exposes this. Fixed in x64 version as well.

http://codereview.chromium.org/335005/diff/1/5
File src/x64/codegen-x64.h (right):

http://codereview.chromium.org/335005/diff/1/5#newcode638
Line 638: // Flag that indicates whether how to generate code for the stub.
On 2009/10/26 11:23:35, Kevin Millikin wrote:
> Extra 'whether'.

Done. Fixed in ia32 version as well.