IonMonkey: Eliminate negative zero checks for a*a

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dvander, Assigned: terrence)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Created attachment 562892 [details]
benchmark

In the attached benchmark, we could get a significant speedup from removing the negative zero check on the multiplication. We should be able to do this in MIR by seeing that (a * a) will never be negative.
(Assignee)

Comment 1

6 years ago
Created attachment 563502 [details] [diff] [review]
First attempt.

Speedup is 1.12x, but I need some feedback on the patch's specifics:

1) I think isCongruent is a more specific (and costly) test that we want for this, but I could be quite wrong.

2) Is there a way to implement this check without having to copy the body of MBinaryArithInstruction::foldsTo?

3) Should I add a setter for canBeNegativeZero_?
Assignee: general → tcole
Status: NEW → ASSIGNED
Attachment #563502 - Flags: feedback?(dvander)
(Assignee)

Comment 2

6 years ago
Created attachment 563524 [details] [diff] [review]
v2

Shorter patch using congruentTo and calling out to super (requires us to re-acquire lhs/rhs).  It seems to be epsilon slower than v1 in the benchmark, but it's well within the error bars.
Attachment #563524 - Flags: feedback?(dvander)
Comment on attachment 563524 [details] [diff] [review]
v2

Nice.
Attachment #563524 - Flags: feedback?(dvander) → review+
Attachment #563502 - Attachment is obsolete: true
Attachment #563502 - Flags: feedback?(dvander)
(Assignee)

Comment 4

6 years ago
Created attachment 567918 [details] [diff] [review]
rebased

I still don't have commit access, but that's no reason to let this bitrot.
Attachment #563524 - Attachment is obsolete: true
Attachment #567918 - Flags: checkin?(dvander)
Attachment #567918 - Flags: checkin?(dvander)
https://hg.mozilla.org/projects/ionmonkey/rev/80f0dce7aa2e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.