716694 - IonMonkey: Extend JSOP_MOD to work with float arguments

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Eddy Bruel [:ejpbruel]]

Attachment: Move double result to xmm0 on x86

As a first step, this patch arranges for the result of an ABI call to be transferred from the x87 stack to the xmm0 register (if the result is a double, and we're on x86, the x64 ABI already puts the result in xmm0 by default).

Before I continue, can somebody take a look at this patch and see if it makes sense?

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 587910 [details] [diff] [review]
Move double result to xmm0 on x86

Review of attachment 587910 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/x86/MacroAssembler-x86.h
@@ +74,5 @@
>      // callee.  It returns the space which has to be allocated for calling the
>      // function.
>      //
>      // arg            Number of arguments of the function.
> +    uint32 setupABICall(uint32 arg, bool resultIsDouble = false);

Two suggestions - could this be moved to callWithABI? then you don't have to save it locally, and don't need to change stack computation (you'd just reserve/free in the same place).
It'd also be nice to have an enum input instead of a boolean (it's hard to tell what a boolean means). Like Result_Normal, Result_Double or something.

Comment 3

[Paul Wright]

What is the status here?

Comment 4

[Eddy Bruel [:ejpbruel]]

(In reply to Paul Wright from comment #3)
> What is the status here?

I'm working on this bug. It's currently blocked on bug 709423: we want to use the fmod function to implement the floating-point version of JSOP_MOD, but for this to work, the callWithABI API needs to be able to pass double arguments to a function, and return a double result. I just filed a patch that does exactly that for x86 and x64. Once it gets r+'d, I intend to finish this the implementation of JSOP_MOD for those platforms. The plan is to open a separate bug for the ARM version.

Hopes that answers your question.

Comment 5

[Paul Wright]

Yes, it does.  Thank you!

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Patch

Pretty straight-forward now that we can use doubles with callWithAbi. With --no-jm this wins about 5-10% on v8-raytrace.

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Patch

Now marked as call instruction..

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/projects/ionmonkey/rev/83c83b185199