IonMonkey: Extend JSOP_MOD to work with float arguments

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ejpbruel, Assigned: jandem)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ion:p1:fx18])

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Updated

5 years ago
Blocks: 695770
Depends on: 709423
(Reporter)

Comment 1

5 years ago
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?
Attachment #587910 - Flags: review?(dvander)
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.
Attachment #587910 - Flags: review?(dvander) → review+

Comment 3

5 years ago
What is the status here?
(Reporter)

Comment 4

5 years ago
(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

5 years ago
Yes, it does.  Thank you!
(Assignee)

Comment 6

5 years ago
Created attachment 645302 [details] [diff] [review]
Patch

Pretty straight-forward now that we can use doubles with callWithAbi. With --no-jm this wins about 5-10% on v8-raytrace.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #645302 - Flags: review?(dvander)
(Assignee)

Comment 7

5 years ago
Created attachment 646089 [details] [diff] [review]
Patch

Now marked as call instruction..
Attachment #645302 - Attachment is obsolete: true
Attachment #645302 - Flags: review?(dvander)
Attachment #646089 - Flags: review?(dvander)
Attachment #646089 - Flags: review?(dvander) → review+
Whiteboard: [ion:p1:fx18]
http://hg.mozilla.org/projects/ionmonkey/rev/83c83b185199
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.