1076922 - IonMonkey: Implement RToFloat32 Recover Instruction

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp] {away}]

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.

Comment 1

[Jyri Pyykkönen]

Attachment: Implement the RToFloat32 instruction

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.

Comment 2

[Nicolas B. Pierron [:nbp] {away}]

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.

Comment 3

[Nicolas B. Pierron [:nbp] {away}]

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.

Comment 4

[Nicolas B. Pierron [:nbp] {away}]

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?

Comment 5

[Jyri Pyykkönen]

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.

Comment 6

[Nicolas B. Pierron [:nbp] {away}]

(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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/27d545252264