x64 code generation for construct calls, declaring global variables...

x64 code generation for construct calls, declaring global variables
and for runtime calls.

We could not handle functions with no explicit return statement.  I
added support for that as well.  The place was hard to find because
code was left out from the codegenerator with no TODO comment.  We
need to make sure to comment if we leave out code when porting
something. :-)


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

[William Hesse, 2009-06-24 07:55:57]

LGTM.

http://codereview.chromium.org/146029/diff/2001/2006
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/146029/diff/2001/2006#newcode205
Line 205: __ EnterConstructFrame();
What is the calling convention of this function on entry, and also after
EnterConstructFrame?

http://codereview.chromium.org/146029/diff/2001/2006#newcode227
Line 227: __ movq(rbx, rax);  // store result in ebx
rbx, not ebx

http://codereview.chromium.org/146029/diff/2001/2006#newcode230
Line 230: // ebx: newly allocated object
rbx

http://codereview.chromium.org/146029/diff/2001/2006#newcode253
Line 253: __ push(Operand(rbx, rcx, times_pointer_size, 0));
Isn't there a three-argument version of Operand(Register, Register,
ScaleFactor)?  If not, we should create one, I think.

http://codereview.chromium.org/146029/diff/2001/2006#newcode276
Line 276: __ CmpInstanceType(rcx, FIRST_JS_OBJECT_TYPE);
CmpInstanceType can now be written inline as a single instruction,
cmpb(Operand(rcx, typeOffset), Immediate(FIRST_JS_OBJECT_TYPE))
Update CmpInstanceType, or else write instructions directly.
I have just put a bunch of these in - maybe they should all be changed to
CmpInstanceType().

http://codereview.chromium.org/146029/diff/2001/2008
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/146029/diff/2001/2008#newcode1403
Line 1403: Result temp = allocator()->Allocate();
Use kScratchRegister here.

http://codereview.chromium.org/146029/diff/2001/2008#newcode1647
Line 1647: __ testl(value.reg(), Immediate(kSmiTagMask));
We have many of these throughout the code, and have been using testq for them. 
Since the high bits of the mask are zero, it doesn't matter which we use, but
testl can lead to a shorter encoding, I suppose.  We should be consistent, for
now.  If you plan to replacing these throughout all the code, that is OK with
me.

[Mads Ager (chromium), 2009-06-24 08:18:50]

Thanks for the comments!

http://codereview.chromium.org/146029/diff/2001/2006
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/146029/diff/2001/2006#newcode205
Line 205: __ EnterConstructFrame();
On 2009/06/24 07:55:57, William Hesse wrote:
> What is the calling convention of this function on entry, and also after
> EnterConstructFrame?

rax: number of arguments
rdi: constructor function

http://codereview.chromium.org/146029/diff/2001/2006#newcode227
Line 227: __ movq(rbx, rax);  // store result in ebx
On 2009/06/24 07:55:57, William Hesse wrote:
> rbx, not ebx

Done.

http://codereview.chromium.org/146029/diff/2001/2006#newcode230
Line 230: // ebx: newly allocated object
On 2009/06/24 07:55:57, William Hesse wrote:
> rbx

Done.

http://codereview.chromium.org/146029/diff/2001/2006#newcode253
Line 253: __ push(Operand(rbx, rcx, times_pointer_size, 0));
On 2009/06/24 07:55:57, William Hesse wrote:
> Isn't there a three-argument version of Operand(Register, Register,
> ScaleFactor)?  If not, we should create one, I think. 

There isn't no, and I'm not sure it adds much to have it.

http://codereview.chromium.org/146029/diff/2001/2006#newcode276
Line 276: __ CmpInstanceType(rcx, FIRST_JS_OBJECT_TYPE);
On 2009/06/24 07:55:57, William Hesse wrote:
> CmpInstanceType can now be written inline as a single instruction,
> cmpb(Operand(rcx, typeOffset), Immediate(FIRST_JS_OBJECT_TYPE))
> Update CmpInstanceType, or else write instructions directly.
> I have just put a bunch of these in - maybe they should all be changed to
> CmpInstanceType().

CmpInstanceType does the one instruction compare.  Here I should use
CompareObjectType though to take care of the map load as well.

http://codereview.chromium.org/146029/diff/2001/2008
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/146029/diff/2001/2008#newcode1403
Line 1403: Result temp = allocator()->Allocate();
On 2009/06/24 07:55:57, William Hesse wrote:
> Use kScratchRegister here.

Done.

http://codereview.chromium.org/146029/diff/2001/2008#newcode1647
Line 1647: __ testl(value.reg(), Immediate(kSmiTagMask));
On 2009/06/24 07:55:57, William Hesse wrote:
> We have many of these throughout the code, and have been using testq for them.

> Since the high bits of the mask are zero, it doesn't matter which we use, but
> testl can lead to a shorter encoding, I suppose.  We should be consistent, for
> now.  If you plan to replacing these throughout all the code, that is OK with
> me.

We were using it inconsistently before.  There was a mix of the two.  I have
changed it to consistently use testl in all the x64 files.