Porting binary op ICs to x64.

Porting binary op ICs to x64.

[Vladislav Kaznacheev, 2010-03-11 15:33:12]

The gain on Sunspider-sum is ~0.3%

[Mads Ager (chromium), 2010-03-17 10:10:59]

LGTM, with a couple of nits.

Kevin is out of town at the moment.  The port looks correct to me.

We need to get rid of the relocation info hack.  I'm fine with doing that in a
separate change, but we need to do it.  Can we measure if there is any visible
impact of not resetting the binary operation ICs at GC time?

http://codereview.chromium.org/845002/diff/4001/3002
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/845002/diff/4001/3002#newcode8747
src/x64/codegen-x64.cc:8747: if (op_ == Token::ADD) {
Indentation is off here.

http://codereview.chromium.org/845002/diff/4001/3002#newcode8919
src/x64/codegen-x64.cc:8919: // Generate an unreachable reference to the DEFAULT
stub so that it can be
This still feels like a nasty hack to me.  Does the clearing matter?  We need to
experiment with this and get rid of this hack somehow.

http://codereview.chromium.org/845002/diff/4001/3002#newcode8964
src/x64/codegen-x64.cc:8964: // edx, eax.
ia32 register names.

http://codereview.chromium.org/845002/diff/4001/3002#newcode9020
src/x64/codegen-x64.cc:9020: HandleScope scope;
This returns a handle from a destroyed handle scope which is not GC safe.  The
caller of GetBinaryOpStub already has a handle scope, so just remove this one.

[Vladislav Kaznacheev, 2010-03-18 11:14:23]

http://codereview.chromium.org/845002/diff/4001/3002
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/845002/diff/4001/3002#newcode8747
src/x64/codegen-x64.cc:8747: if (op_ == Token::ADD) {
On 2010/03/17 10:10:59, Mads Ager wrote:
> Indentation is off here.

Done.

http://codereview.chromium.org/845002/diff/4001/3002#newcode8919
src/x64/codegen-x64.cc:8919: // Generate an unreachable reference to the DEFAULT
stub so that it can be
Added a TODO to experiment and remove.
On 2010/03/17 10:10:59, Mads Ager wrote:
> This still feels like a nasty hack to me.  Does the clearing matter?  We need
to
> experiment with this and get rid of this hack somehow.

http://codereview.chromium.org/845002/diff/4001/3002#newcode8964
src/x64/codegen-x64.cc:8964: // edx, eax.
On 2010/03/17 10:10:59, Mads Ager wrote:
> ia32 register names.

Done.

http://codereview.chromium.org/845002/diff/4001/3002#newcode9020
src/x64/codegen-x64.cc:9020: HandleScope scope;
On 2010/03/17 10:10:59, Mads Ager wrote:
> This returns a handle from a destroyed handle scope which is not GC safe.  The
> caller of GetBinaryOpStub already has a handle scope, so just remove this one.

Done.