Closed Bug 1020467 Opened 6 years ago Closed 6 years ago

Nuke allowFloat32Optimizations()

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: bbouvier, Assigned: bbouvier)

Details

Attachments

(1 file)

This was introduced when waiting for ARM backend implementation. Now that all architectures (including MIPS!) support Float32, we can just get rid of it.

04.18:59:52 < bbouvier> has anybody tried to disable allowFloat32Optimization() recently, on any platform?
04.19:00:27 < mjrosenb> bbouvier: I did!
04.19:00:31 < mjrosenb> bbouvier: it failed horribly!
04.19:01:12 < bbouvier> mjrosenb: that's unfortunate but kind of expected
04.19:01:23 < bbouvier> mjrosenb: i would like to nuke it entirely, what do you think?
04.19:02:03 < bbouvier> not sure that keeping it and making it work is worth the investment
04.19:05:01  * mjrosenb tries to remember *why* he turned it off...
04.19:05:07 <    mjrosenb>| I'm pretty sure I had a good reason.
04.19:06:54 < bbouvier> the only reason that justifies keeping it would be to make sure whether a bug is due to float32 or not
04.19:07:19 < bbouvier> but in that case, i'd extend the concept to have a shell flag to be able to enable / disable float32 optimizations
04.19:07:32 < bbouvier> and once again, not feeling we need that
04.19:08:39 <    mjrosenb>| right.  and even embedders don't have a reason to disable it at all.

Unless somebody is in the mood for making it a shell flag and fix the issues that need to be fixed, I'd be in favor with deleting it entirely.
If everybody agrees, let's go.
Attachment #8434309 - Flags: review?(sstangl)
Comment on attachment 8434309 [details] [diff] [review]
Nuke allowFloat32Optimizations()

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

Better ARM support landed yesterday on master, and AArch64 support is trivial due to register sanity, so this should be good to go!
Attachment #8434309 - Flags: review?(sstangl) → review+
It seems that full float32 support for ARM ended landing today, so try-ing just to be sure, as the patch had time to bit rot a bit.

https://tbpl.mozilla.org/?tree=Try&rev=08f2555a8163
https://hg.mozilla.org/mozilla-central/rev/6c88cacf4e75
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.