1492064 - Baseline JIT is not disabled on unsupported variants of supported CPUs

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Mike Hommey [:glandium]]

While Mozilla.org builds of Firefox dropped support for x86 CPUs without SSE2, downstreams may support it. This is generally handled gracefully by having runtime detection, and the ION JIT does it. But the baseline JIT doesn't.

I have a WIP patch that adds a cx->runtime()->jitSupportsFloatingPoint condition to IsBaselineEnabled but I have a few interrogations:
- What does it mean for non x86 CPUs? (like ARM, etc.)
- It seems WASM does the right thing in wasm::HasCompilerSupport, but I don't know if it's used in all code paths.

Not sure who would have the best answers for this. Trying Jan.

Comment 1

[Lars T Hansen [:lth]]

wasm::HasCompilerSupport should properly gate wasm always, and then there are additional checks on hardware capabilities in wasm::BaselineCanCompile and wasm::IonCanCompile to determine if specific compilers are available.  For example, the baseline compiler requires IDIV on ARM, and both compilers require LDREXD / STREXD on ARM for atomics support.

Comment 2

[Jan de Mooij [:jandem]]

Baseline should work fine without floating point support (at least that's how it used to work but maybe someone broke it since then).

I'd really like to require SSE2+ and ARMv7+ at some point in the JIT. If people want to run Firefox on ancient hardware they can compile with --disable-ion (and use the none/ backend, no JITs). I honestly don't think it's worth our time and resources fixing what are effectively tier 3 platforms.

That said, adding a cx->runtime()->jitSupportsFloatingPoint check to IsBaselineEnabled seems OK.

Comment 3

[Mike Hommey [:glandium]]

> If people want to run Firefox on ancient hardware they can compile with --disable-ion (and use the none/ backend, no JITs).

For downstreams such as Debian, it's preferable to keep a runtime way to disable JITs on unsupported CPUs, allowing the same binary to run both on systems with and without SSE2, with a JIT for the former.

Comment 4

[Jan de Mooij [:jandem]]

FWIW we used to have a --no-fpu shell flag (to pretend we don't have SSE2) to find bugs here. We removed it in bug 1279529, two years ago, because we thought all of our builds would require SSE2...

Anyway, suggested fix SGTM, if this is causing crashes downstream.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Bug 1492064 - Disable baseline JIT when SSE2 is not supported at runtime

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 9010118 [details]
Bug 1492064 - Disable baseline JIT when SSE2 is not supported at runtime

Jan de Mooij [:jandem] has approved the revision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/921e4aa179f8
Disable baseline JIT when SSE2 is not supported at runtime r=jandem

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/921e4aa179f8

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 9010118 [details]
Bug 1492064 - Disable baseline JIT when SSE2 is not supported at runtime

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Necessary for some downstreams.
User impact if declined: Downstreams such as Debian provide OSes that work on older x86 machines, and as such, try to keep Firefox working on those machines. For users with such machines (and there are surprisingly lots of them), Firefox crashes without this patch.
Fix Landed on Version: 64
Risk to taking this patch (and alternatives if risky): For Firefox builds that Mozilla ships, the patch essentially is a no-op: the added condition is true on all machines with SSE2, which is the minimum supported for those builds. On arm, it means machines with VFP, which is all those we support for Android. For downstreams like Debian, this just ensures SSE2 instructions are not used on machines without SSE2. Note that Ion jit is already disabled for such machines, this disables the baseline jit for them.
String or UUID changes made by this patch: N/A

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9010118 [details]
Bug 1492064 - Disable baseline JIT when SSE2 is not supported at runtime

Fix for some downstream distros. Approved for ESR 60.3.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/528cabdd9665