Bug 1336139 comment 9 shows a subtle issue with the x87 floating-point unit being set to use 53 bits of mantissa in doubles (64 bits doubles), instead of the maximum 64 bits (80 bits doubles) that C/C++ can access. I think this was introduced because there could be floating-point differences between the interpreter (which was using the x87 FP unit by default) and the JIT (which is using SSE2 instructions by default), in the past. Now that we have SSE2 enabled by default on all x86 platforms, there should not be any differences anymore because of this. So we can re-enable 80 bits of precision for the FP unit. It's also nicer to embedders, because the default seems to be 80 bits of precision on a C program, so their results wouldn't be affected by our setting. See also bug 1336139 comment 10. Jan, choosing you as a reviewer because this seems to affect more particularly the JITs, but feel free to redirect if you think someone else would be more appropriate. CC'ing a few people who could care about the change or would have an opinion to add here.

Comment on attachment 8840541 [details] [diff] [review] Remove FIX_FPU Review of attachment 8840541 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense, hopefully this won't affect performance or differential fuzzing.

Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/06254f3037c3 Enable 80-bits precision for the x87 floating-point unit; r=jandem

Reopening: on Windows, the default is to have 64 bits of precision, and not 80 bits as we'd like to.

This backs out the previous patch, and forces the control word to using 80 bits precision, on 32-bits unix *and* win32, which has been an issue has shown in a try build. Not asking for review yet, since I want to make sure the blocking bug is fixed with this patch.

Chris, Monty: picked you as I saw your names in the blame log of http://searchfox.org/mozilla-central/source/media/libvorbis/lib/os.h#102 and http://searchfox.org/mozilla-central/source/media/libvpx/libvpx/vpx_ports/x86.h#274. Does it matter to you if we change the FPU precision mode to 80 bits instead of 64 bits, in Gecko in general (necessary for a change in Spidermonkey for wasm)? It seems libvorbis and libvpx set/reset the control word in sub-scopes, but just wanted to make sure it is fine to do so.

Probably OK, but Monty should have final say-so regarding libvorbis and I think Tim is a better person to comment on libvpx. My main concern is whether the FPU precision switch is per-thread or per-process. If, for example, we're in the middle of doing some JS FPU calculations, and libvorbis or libvpx (which runs off-main-thread) switches the precision, would that interfere with the calculation that JS was doing and thus cause an inconsistent result? I asked dmajor, and he tried setting the FPU mode in WinDBG on one thread and other threads didn't see the change, so it appears that (at least on Windows) it's per-thread. So we're probably OK to make this change. But I think Tim and Monty should approve this first.

My understanding is that libvpx uses this to ensure the encoder produces the same results on both 32-bit and 64-bit platforms. We don't have any tests that require this AFAIK (and it will continue to be tested upstream), and since I believe we now require SSE support, in the worst case we can build these files with -mfpmath=sse and achieve the same result. libvpx does not use very many libc math functions in the encoder (which might still use x87 internally, and likely are overriding the control word themselves to achieve the precision they require if they do). From a quick audit, just sqrt(), and only in the first pass of multi-pass encoding, which I do not believe is something we use or expose. So I don't see any issues, and there's a mitigation path in case I'm wrong.

Actually we can close; I've found another way forward for bug 1336139.