Implement hardfloat calling convention in macro assembler and simulator.

Implement hardfloat calling convention in macro assembler and simulator.

[Søren Thygesen Gjesse, 2011-04-27 08:19:01]

LGTM

http://codereview.chromium.org/6874007/diff/5010/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5010/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:833: __ PrepareCallCFunction(4, scratch);  // Two
doubles are 4 arguments.
We should probably pass 0 instead of 4 here for hardfloat. Maybe we should try
to change PrepareCallCFunction to communicate the signature better, but leave
that for a separate change.

This goes for all PrepareCallCFunction below.

http://codereview.chromium.org/6874007/diff/5010/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:834: if (FLAG_hardfloat) {
As far as I can see CallCCodeForDoubleOperation is only used if VFP3 is not
available. How about asserting this and not support --novfp3 --hardfloat?

http://codereview.chromium.org/6874007/diff/5010/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:2845: if (FLAG_hardfloat) {
I think we should consider changing the TranscendentalCacheStub to take the
argument in d0 instead of d2 to avoid this move. But again leave it for a
separate change.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:842: if (FLAG_hardfloat) {
Please check whether dst is d0.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2807: int stack_passed_arguments = 0;
I am not sure this calculation is 100% correct. If double arguments are
interleaved with integer arguments then more space is needed as each double
argument on the stack needs to be 8-byte aligned. To make this calculation I
think we need some signature information.

For now maybe just update the comment in the .h file to state that double
aguments are always expected to be before core register arguments.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2845: if (FLAG_hardfloat) {
Please check if dreg is d0. Maybe add VMove to the macro assembler like Move for
core registers.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2855: if (FLAG_hardfloat) {
Ditto.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2867: if (FLAG_hardfloat) {
Ditto (also mov -> Move).

http://codereview.chromium.org/6874007/diff/5010/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5010/src/arm/simulator-arm.cc#new...
src/arm/simulator-arm.cc:1738: if (::v8::internal::FLAG_trace_sim ||
!stack_aligned) {
This printout of the arguments is not correct. It was also wrong before, but now
you have more information to print it correctly.

[Karl Klose, 2011-04-27 12:54:13]

I have uploaded a new version.

http://codereview.chromium.org/6874007/diff/5010/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5010/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:833: __ PrepareCallCFunction(4, scratch);  // Two
doubles are 4 arguments.
Changed to PrepareCallCFunction(0, 2, scratch).

http://codereview.chromium.org/6874007/diff/5010/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:834: if (FLAG_hardfloat) {
It is used from the TRBO stub, also with VFP3.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:842: if (FLAG_hardfloat) {
I replaced the vmov with the new MacroAssembler::Move.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2807: int stack_passed_arguments = 0;
I have added the comment.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2845: if (FLAG_hardfloat) {
Used Move.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2855: if (FLAG_hardfloat) {
On 2011/04/27 08:19:01, Søren Gjesse wrote:
> Ditto.

Done.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:2867: if (FLAG_hardfloat) {
On 2011/04/27 08:19:01, Søren Gjesse wrote:
> Ditto (also mov -> Move).

Done.

http://codereview.chromium.org/6874007/diff/5010/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5010/src/arm/simulator-arm.cc#new...
src/arm/simulator-arm.cc:1738: if (::v8::internal::FLAG_trace_sim ||
!stack_aligned) {
On 2011/04/27 08:19:01, Søren Gjesse wrote:
> This printout of the arguments is not correct. It was also wrong before, but
now
> you have more information to print it correctly.

Done.

[Søren Thygesen Gjesse, 2011-04-27 13:26:03]

LGTM

http://codereview.chromium.org/6874007/diff/5013/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5013/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:841: __
CallCFunction(ExternalReference::double_fp_operation(op, masm->isolate()),
The number 4 passed here should be two values as in PrepareCallCFunction, and
match that.

[Karl Klose, 2011-04-27 14:27:56]

http://codereview.chromium.org/6874007/diff/5013/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/6874007/diff/5013/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:841: __
CallCFunction(ExternalReference::double_fp_operation(op, masm->isolate()),
On 2011/04/27 13:26:03, Søren Gjesse wrote:
> The number 4 passed here should be two values as in PrepareCallCFunction, and
> match that.

Done.