Make assembler-x64.h and assembler-x64-inl.h compile

Make assembler-x64.h and assembler-x64-inl.h compile

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

[William Hesse, 2009-05-06 09:59:45]

Compiling versions of assembler-x64.h

[Dean McNamee, 2009-05-06 10:10:38]

http://codereview.chromium.org/115021/diff/1005/1007
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/115021/diff/1005/1007#newcode27
Line 27: 
we should have guards even on -inl.h files.

http://codereview.chromium.org/115021/diff/1005/1007#newcode36
Line 36: return 0;
return NULL

http://codereview.chromium.org/115021/diff/1005/1006
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/115021/diff/1005/1006#newcode37
Line 37: #ifndef V8_ASSEMBLER_X64_H_
this guard is wrong.

http://codereview.chromium.org/115021/diff/1005/1006#newcode76
Line 76: //    return 1 << code_;
you can just drop this code, we shouldn't check in comment out code.

http://codereview.chromium.org/115021/diff/1005/1006#newcode106
Line 106: bool is_valid() const  { return 0 <= code_ && code_ < 2; }  //
currently
currently ?

http://codereview.chromium.org/115021/diff/1005/1006#newcode928
Line 928: #endif  // V8_ASSEMBLER_IA32_H_
this is wrong

[Lasse Reichstein, 2009-05-06 11:03:11]

LGTM, with Dean's comments. This is a good step towards getting arch=x64 to
compile.

http://codereview.chromium.org/115021/diff/1005/1006
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/115021/diff/1005/1006#newcode76
Line 76: //    return 1 << code_;
I disagree.
If code expected to be close to correct, but not 64-bit compatible yet,
commenting it out and adding UNIMPLEMENTED is a fine way to mark that.
It should only happen in the x64/ files, though.

[William Hesse, 2009-05-06 11:10:35]

http://codereview.chromium.org/115021/diff/1005/1007
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/115021/diff/1005/1007#newcode27
Line 27: 
On 2009/05/06 10:10:38, Dean McNamee wrote:
> we should have guards even on -inl.h files.

Done.

http://codereview.chromium.org/115021/diff/1005/1007#newcode36
Line 36: return 0;
On 2009/05/06 10:10:38, Dean McNamee wrote:
> return NULL

Done.

http://codereview.chromium.org/115021/diff/1005/1006
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/115021/diff/1005/1006#newcode37
Line 37: #ifndef V8_ASSEMBLER_X64_H_
On 2009/05/06 10:10:38, Dean McNamee wrote:
> this guard is wrong.

Done.

http://codereview.chromium.org/115021/diff/1005/1006#newcode76
Line 76: //    return 1 << code_;
I removed it anyway.

On 2009/05/06 11:03:11, Lasse Reichstein wrote:
> I disagree.
> If code expected to be close to correct, but not 64-bit compatible yet,
> commenting it out and adding UNIMPLEMENTED is a fine way to mark that.
> It should only happen in the x64/ files, though.

http://codereview.chromium.org/115021/diff/1005/1006#newcode106
Line 106: bool is_valid() const  { return 0 <= code_ && code_ < 2; }  //
currently
On 2009/05/06 10:10:38, Dean McNamee wrote:
> currently ?

I didn't add this, I guess it means we currently only
use the first two XMM registers.  Removed.

http://codereview.chromium.org/115021/diff/1005/1006#newcode928
Line 928: #endif  // V8_ASSEMBLER_IA32_H_
On 2009/05/06 10:10:38, Dean McNamee wrote:
> this is wrong

Done.

[Dean McNamee, 2009-05-06 11:14:31]

OK with the guards properly fixed.

http://codereview.chromium.org/115021/diff/1011/7
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/115021/diff/1011/7#newcode29
Line 29: #define V8_X64_ASSEMBLER_IA32_INL_H_
this still says IA32, it should be X64

http://codereview.chromium.org/115021/diff/1011/6
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/115021/diff/1011/6#newcode926
Line 926: #endif  // V8_X64_ASSEMBLER_IA32_H_
This still says IA32, it should be X64

[Kevin Millikin (Chromium), 2009-05-06 11:32:12]

http://codereview.chromium.org/115021/diff/1011/6
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/115021/diff/1011/6#newcode35
Line 35: // A light-weight X64 Assembler.
"light-weight" => "lightweight"

http://codereview.chromium.org/115021/diff/1011/6#newcode64
Line 64: // kNumRegisters == 16 is not declared yet.
Why not move its declaration?  Or it actually makes sense just to make it a
static const in struct Register.

http://codereview.chromium.org/115021/diff/1011/6#newcode67
Line 67: // The byte-register distinction of ai32 has dissapeared.
ai32?  If it's disappeared, why not just get rid of the function?

http://codereview.chromium.org/115021/diff/1011/6#newcode78
Line 78: // (unfortunately we can't make this private in a struct)
This is my new favorite comment.

[William Hesse, 2009-05-06 12:07:13]

http://codereview.chromium.org/115021/diff/1011/6
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/115021/diff/1011/6#newcode35
Line 35: // A light-weight X64 Assembler.
On 2009/05/06 11:32:12, Kevin Millikin wrote:
> "light-weight" => "lightweight"

Done.

http://codereview.chromium.org/115021/diff/1011/6#newcode64
Line 64: // kNumRegisters == 16 is not declared yet.
On 2009/05/06 11:32:12, Kevin Millikin wrote:
> Why not move its declaration?  Or it actually makes sense just to make it a
> static const in struct Register.

Done.

http://codereview.chromium.org/115021/diff/1011/6#newcode67
Line 67: // The byte-register distinction of ai32 has dissapeared.
On 2009/05/06 11:32:12, Kevin Millikin wrote:
> ai32?  If it's disappeared, why not just get rid of the function?

I want everyone to keep thinking like you do, whenever they see this line, until
they do get rid of the function, and verify that whatever calls it is fixed to
not use it.

http://codereview.chromium.org/115021/diff/1011/6#newcode926
Line 926: #endif  // V8_X64_ASSEMBLER_IA32_H_
On 2009/05/06 11:14:31, Dean McNamee wrote:
> This still says IA32, it should be X64

Done.

[Dean McNamee, 2009-05-06 12:09:49]

Ok