Last Comment Bug 716694 - IonMonkey: Extend JSOP_MOD to work with float arguments
: IonMonkey: Extend JSOP_MOD to work with float arguments
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
: Jason Orendorff [:jorendorff]
Depends on: 709423
Blocks: 695770
  Show dependency treegraph
Reported: 2012-01-09 14:43 PST by Eddy Bruel [:ejpbruel]
Modified: 2012-07-27 17:20 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Move double result to xmm0 on x86 (6.71 KB, patch)
2012-01-11 18:24 PST, Eddy Bruel [:ejpbruel]
dvander: review+
Details | Diff | Splinter Review
Patch (7.33 KB, patch)
2012-07-24 07:41 PDT, Jan de Mooij [:jandem]
no flags Details | Diff | Splinter Review
Patch (7.55 KB, patch)
2012-07-26 04:29 PDT, Jan de Mooij [:jandem]
dvander: review+
Details | Diff | Splinter Review

Description Eddy Bruel [:ejpbruel] 2012-01-09 14:43:46 PST

Comment 1 Eddy Bruel [:ejpbruel] 2012-01-11 18:24:21 PST
Created attachment 587910 [details] [diff] [review]
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] 2012-01-12 17:08:38 PST
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 2012-02-11 07:56:27 PST
What is the status here?
Comment 4 Eddy Bruel [:ejpbruel] 2012-02-11 08:06:53 PST
(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 2012-02-11 08:10:47 PST
Yes, it does.  Thank you!
Comment 6 Jan de Mooij [:jandem] 2012-07-24 07:41:35 PDT
Created attachment 645302 [details] [diff] [review]

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] 2012-07-26 04:29:42 PDT
Created attachment 646089 [details] [diff] [review]

Now marked as call instruction..
Comment 8 David Anderson [:dvander] 2012-07-27 17:20:48 PDT

Note You need to log in before you can comment on or make changes to this bug.