Closed Bug 1076922 Opened 5 years ago Closed 5 years ago

IonMonkey: Implement RToFloat32 Recover Instruction

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: nbp, Assigned: jyri.pyykkonen, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Implement RToFloat32 in js/src/jit/Recover.cpp.
See Bug 1003801 comment 0 for explanation.

To produce a MToFloat32, write "Math.fround(x)" in the test case.
Here's my implementation of this. I haven't really touched the Mozilla codebase before so I hope the patch is at least going in the right direction. I'm not sure the test cases are correct, but at least there is a gray "tofloat32" in the iongraph output for rtofloat32_number.
Attachment #8507910 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8507910 [details] [diff] [review]
Implement the RToFloat32 instruction

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

Welcome to the Mozillian community :)
The patch looks good, I will remove the trailing white space and send the patch to our continuous integration system.
Attachment #8507910 - Flags: review?(nicolas.b.pierron) → review+
Assignee: nobody → jyri.pyykkonen
Status: NEW → ASSIGNED
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=097acc430a22

This is the link to our try-server, which is where we select tests which are running before merging the changes into mozilla-inbound.  If all of these turn green, then it would be good for landing.
https://hg.mozilla.org/integration/mozilla-inbound/rev/27d545252264

As the Try run is green, for the tests that we are interested in, I pushed the patch to mozilla-inbound.  Later today, a sheriff will merge the branch into mozilla-central and your modification would be available to every Nightly user.

You can follow how it goes on continuous integration system:
https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound

Congratualation :)

Is there any other issue that you want to address?
Thank you for accepting the patch! I was a bit surprised to see it approved on the first try. 

As for the trailing white space, I guess it was on line 76? It took me a couple of reads to even notice that.
(In reply to Jyri Pyykkönen from comment #5)
> Thank you for accepting the patch!

The patch is good, there is no reason to refuse it, and I am forgiving about trailing white space on the first patches ;)

> I was a bit surprised to see it approved
> on the first try. 

If you want to play again, I can find a few bugs with increasing level of difficulties.
Such as Bug 1062888, Bug 1066040 and Bug 1068605.  Otherwise you can have a look at http://www.joshmatthews.net/bugsahoy/?jseng=1
https://hg.mozilla.org/mozilla-central/rev/27d545252264
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.