Introduce StackOperandForArgument for X64 to access stack argument

Introduce StackOperandForArgument for X64 to access stack argument

[haitao.feng, 2013-07-23 14:14:28]

This abstraction uses the new introduced kPCOnStackSize constant and makes the
stack argument access logic (return address size is considered as kPointerSize)
applies for both X64 and X32.

[danno, 2013-07-24 22:32:52]

https://codereview.chromium.org/19857006/diff/1/src/x64/macro-assembler-x64.h
File src/x64/macro-assembler-x64.h (right):

https://codereview.chromium.org/19857006/diff/1/src/x64/macro-assembler-x64.h...
src/x64/macro-assembler-x64.h:1521: inline Operand StackArgumentOperand(int32_t
disp) {
Please just call this StackOperand. There are cases where stack slots may be
accessed that are not arguments, but you still must use this operand factory.

https://codereview.chromium.org/19857006/diff/1/src/x64/macro-assembler-x64.h...
src/x64/macro-assembler-x64.h:1526: inline Operand StackArgumentOperand(Register
index,
Please just call this StackOperand.

https://codereview.chromium.org/19857006/diff/1/src/x64/stub-cache-x64.cc
File src/x64/stub-cache-x64.cc (right):

https://codereview.chromium.org/19857006/diff/1/src/x64/stub-cache-x64.cc#new...
src/x64/stub-cache-x64.cc:1079: __ movq(StackArgumentOperand(1 * kPointerSize),
object_reg);
1 * kPointerSize -> kPointerSize

https://codereview.chromium.org/19857006/diff/1/src/x64/stub-cache-x64.cc#new...
src/x64/stub-cache-x64.cc:1139: __ movq(StackArgumentOperand(1 * kPointerSize),
reg);
1*kPointerSize -> kPointerSize

[haitao.feng, 2013-07-25 04:14:42]

danno, thanks for the review.

In patch set 2, I use kPCOnStackSize directly for stack operand access when
return address is in the disp computation. It is more natural and intuitive than
patch set 1. Comments are also updated.

[haitao.feng, 2013-07-25 06:31:49]

I originally used __q annotation in this place for some reason. Now I use __a
annotation and it should belong to this CL.

https://codereview.chromium.org/19857006/diff/13001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/19857006/diff/13001/src/x64/builtins-x64.cc#n...
src/x64/builtins-x64.cc:721: __ movq(rax, Operand(rsp, kPCOnStackSize +
kPointerSize));
This is not for argument access, so I did not use 1 * kPointerSize.

[danno, 2013-07-25 19:56:18]

I actually prefer your original approach that wraps the operands with utility
functions, I think it makes it clearer at the caller site what you are trying to
accomplish, the inline offset math just confuses things.

Perhaps you just need more helper functions to make the different cases clear,
e.g.

StackOperand <-- two variants for arguments and indexed access
StackOpeandForPC  <-- returns an operand for the return address on the stack

[haitao.feng, 2013-07-26 02:39:18]

Danno, thanks a lot for your review.

I choose the name StackOperandForArgument as either the "rsp + disp" or "rsp +
index * scale + disp" in this CL are used to access argument. Receiver is
considered as argument 0. For non-argument access, I used kPCOnStackSize instead
of kPointerSize and explained them in the place.

I will use StackOperandForPc to address the return address access in another CL.

https://codereview.chromium.org/19857006/diff/23001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/19857006/diff/23001/src/x64/builtins-x64.cc#n...
src/x64/builtins-x64.cc:711: 
Use kPCOnStackSize instead of kPointerSize.

https://codereview.chromium.org/19857006/diff/23001/src/x64/builtins-x64.cc#n...
src/x64/builtins-x64.cc:720: 
Use kPCOnStackSize instead of kPointerSize.

https://codereview.chromium.org/19857006/diff/23001/src/x64/stub-cache-x64.cc
File src/x64/stub-cache-x64.cc (right):

https://codereview.chromium.org/19857006/diff/23001/src/x64/stub-cache-x64.cc...
src/x64/stub-cache-x64.cc:1077: 
Use kPCOnStackSize instead of kPointerSize.

https://codereview.chromium.org/19857006/diff/23001/src/x64/stub-cache-x64.cc...
src/x64/stub-cache-x64.cc:1137: 
Use kPCOnStackSize instead of kPointerSize.

[danno, 2013-07-30 10:59:17]

Thanks for addressing comments, however, I can't review this patch, since I get
"error: old chunk mismatch" for all of the files. Can you please close this
issue and open a new one with the latest diff?

[haitao.feng, 2013-07-30 12:18:15]

Closed this issue and re-upload to https://codereview.chromium.org/21123008/.