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#newcode833
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#newcode834
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#newcode2845
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.cc#newcode842
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.cc#newcode2807
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.cc#newcode2845
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.cc#newcode2855
src/arm/macro-assembler-arm.cc:2855: if (FLAG_hardfloat) {
Ditto.

http://codereview.chromium.org/6874007/diff/5010/src/arm/macro-assembler-arm.cc#newcode2867
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#newcode1738
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.

http://codereview.chromium.org/6874007/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to