Complete the full codegenerator on x64.

Complete the full codegenerator on x64.


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

[Mads Ager (chromium), 2010-05-20 09:19:43]

http://codereview.chromium.org/2078022/diff/1/6
File src/x64/full-codegen-x64.cc (right):

http://codereview.chromium.org/2078022/diff/1/6#newcode1522
src/x64/full-codegen-x64.cc:1522: case KEYED_PROPERTY: {
This case will be cleaned up once the keyed store ic change lands to make the
calling convention the same as on ia32.

[William Hesse, 2010-05-20 13:11:59]

LGTM, with changes.

http://codereview.chromium.org/2078022/diff/1/6
File src/x64/full-codegen-x64.cc (right):

http://codereview.chromium.org/2078022/diff/1/6#newcode232
src/x64/full-codegen-x64.cc:232: 
Is there a reason not to insert this function in the same position it appears in
in the ia32 implementation, and in the order it appears in in the header file?

http://codereview.chromium.org/2078022/diff/1/6#newcode2028
src/x64/full-codegen-x64.cc:2028: __ CmpInstanceType(rbx, FIRST_JS_OBJECT_TYPE);
These should use above and below, unless you comment that you really want to
interpret non-string types as negative numbers.  The equivalent ia32 code should
use CmpInstanceType, as well, or else this code should be changed to match the
ia32 code.  And the ia32 code should use below and above, not less and greater.

http://codereview.chromium.org/2078022/diff/1/6#newcode2216
src/x64/full-codegen-x64.cc:2216: __ j(less, &null);
Use unsigned comparison, not signed.  Also on ia32.

http://codereview.chromium.org/2078022/diff/1/7
File src/x64/macro-assembler-x64.cc (right):

http://codereview.chromium.org/2078022/diff/1/7#newcode924
src/x64/macro-assembler-x64.cc:924: Move(kScratchRegister, constant);
Assert that dst is not kScratchRegister?

[Mads Ager (chromium), 2010-05-20 13:49:56]

http://codereview.chromium.org/2078022/diff/1/6
File src/x64/full-codegen-x64.cc (right):

http://codereview.chromium.org/2078022/diff/1/6#newcode232
src/x64/full-codegen-x64.cc:232: 
On 2010/05/20 13:11:59, William Hesse wrote:
> Is there a reason not to insert this function in the same position it appears
in
> in the ia32 implementation, and in the order it appears in in the header file?

No, done.

http://codereview.chromium.org/2078022/diff/1/6#newcode2028
src/x64/full-codegen-x64.cc:2028: __ CmpInstanceType(rbx, FIRST_JS_OBJECT_TYPE);
On 2010/05/20 13:11:59, William Hesse wrote:
> These should use above and below, unless you comment that you really want to
> interpret non-string types as negative numbers.  The equivalent ia32 code
should
> use CmpInstanceType, as well, or else this code should be changed to match the
> ia32 code.  And the ia32 code should use below and above, not less and
greater.

Good catch on using below instead of less. Fixed in classic as well as full
codegen on ia32 and x64. Made the code consistent between the platforms as well.

http://codereview.chromium.org/2078022/diff/1/6#newcode2216
src/x64/full-codegen-x64.cc:2216: __ j(less, &null);
On 2010/05/20 13:11:59, William Hesse wrote:
> Use unsigned comparison, not signed.  Also on ia32.

Done. Also, I changed quite a few other signed instance type comparisons on ia32
to unsigned comparisons as well.

http://codereview.chromium.org/2078022/diff/1/7
File src/x64/macro-assembler-x64.cc (right):

http://codereview.chromium.org/2078022/diff/1/7#newcode924
src/x64/macro-assembler-x64.cc:924: Move(kScratchRegister, constant);
On 2010/05/20 13:11:59, William Hesse wrote:
> Assert that dst is not kScratchRegister?

Done.