1675385 - Differences between x86-64 and arm64 mac wasm prefs prevent universal builds

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Mike Hommey [:glandium]]

The following differences make it impossible to create universal builds with both x86-64 and arm64 support for mac:

--- /tmp/x86/Firefox Nightly.app/Contents/Resources/greprefs.js
+++ /tmp/arm64/Firefox Nightly.app/Contents/Resources/greprefs.js
@@ -463,18 +463,16 @@
 pref("javascript.options.wasm_trustedprincipals", true);
 pref("javascript.options.wasm_verbose",           false);
 pref("javascript.options.wasm_baselinejit",       true);
-//@line 1114 "$SRCDIR/modules/libpref/init/all.js"
-    pref("javascript.options.wasm_cranelift",     false);
-//@line 1116 "$SRCDIR/modules/libpref/init/all.js"
-  pref("javascript.options.wasm_ionjit",          true);
+//@line 1109 "$SRCDIR/modules/libpref/init/all.js"
+    pref("javascript.options.wasm_cranelift",     true);
+//@line 1111 "$SRCDIR/modules/libpref/init/all.js"
+  pref("javascript.options.wasm_ionjit",          false);
 //@line 1118 "$SRCDIR/modules/libpref/init/all.js"
 //@line 1120 "$SRCDIR/modules/libpref/init/all.js"
   pref("javascript.options.wasm_reftypes",        true);
   pref("javascript.options.wasm_gc",              false);
 //@line 1124 "$SRCDIR/modules/libpref/init/all.js"
   pref("javascript.options.wasm_multi_value",     true);
-//@line 1128 "$SRCDIR/modules/libpref/init/all.js"
-    pref("javascript.options.wasm_simd",            true);
 //@line 1133 "$SRCDIR/modules/libpref/init/all.js"
 pref("javascript.options.native_regexp",    true);
 pref("javascript.options.parallel_parsing", true);

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Lars, Steven, pretty urgent request for an Apple Silicon blocker.
Could you please help with that? Thanks

Comment 2

[Lars T Hansen [:lth]]

OK, we can probably fix this by just renaming the switch as 'javascript.options.wasm_optimizing' (as we have made that change elsewhere) and making the correct interpretation internally. We will for sure want to disable cranelift on non-arm64 platforms for this to work smoothly (for now), but we want to do that anyway (bug 1674722) so no big deal.

"Pretty urgent" => "P1". I'll take a look this morning.

Comment 3

[Lars T Hansen [:lth]]

We'll do the work over in the other bug.

Comment 4

[Lars T Hansen [:lth]]

Attachment: Bug 1675385 - Fix moz.configure setup for cranelift. r?rhunt

We want to enable cranelift on aarch64 hardware except on Windows (due
to ABI issues) and on x64 with the aarch64 simulator, but not on x64
in general (because it is immature).

Add a test case to ensure that cranelift is not enabled where we do
not expect it to be.

Comment 5

[Lars T Hansen [:lth]]

Attachment: Bug 1675385 - Fix prefs, switches, and selection for cranelift. r?rhunt

This patch makes cranelift and ion exclusive of each other: enabling
cranelift on a platform will effectively disable Ion on that platform.

Specifically there's a change at the pref/switch level that does not
go terribly deep:

• the new about:config option is javascript.options.wasm_optimizingjit,
  the old wasm_cranelift and wasm_ionjit are no more
• new values of X in --wasm-compiler=X in the js shell are 'optimizing'
  and 'baseline+optimizing', the old values 'cranelift', 'ion',
  'baseline+ion' and 'baseline+optimizing' are no more
• we keep the separate prefs internally in the code for ion and cranelift
  but if ENABLE_WASM_CRANELIFT is defined then we never set the ion
  pref to true, and if it is not defined then we never set the cranelift
  pref to true

The trickiest changes are win testWasm.cpp and in the JIT compiler option
processing in jsapi.cpp.

People who will suffer as a result of this are those who are working
on ports of cranelift to new platforms in Firefox. As of now we have
no such work going on.

In the longer term the exclusive-or situation can be alleviated by a
switch that lets cranelift override ion when cranelift is present and
the switch is on. Patches welcome.

Depends on D96058

Comment 6

[Lars T Hansen [:lth]]

Groan, wrong bug.

Comment 7

[Phabricator Automation]

Comment on attachment 9186015 [details]
Bug 1675385 - Fix moz.configure setup for cranelift. r?rhunt

Revision D96058 was moved to bug 1674722. Setting attachment 9186015 [details] to obsolete.

Comment 8

[Phabricator Automation]

Comment on attachment 9186017 [details]
Bug 1675385 - Fix prefs, switches, and selection for cranelift. r?rhunt

Revision D96059 was moved to bug 1674722. Setting attachment 9186017 [details] to obsolete.

Comment 9

[Mike Hommey [:glandium]]

Bug 1674722 still leaves us with this difference:

--- x64/Firefox Nightly.app/Contents/Resources/greprefs.js
+++ aarch64/Firefox Nightly.app/Contents/Resources/greprefs.js
@@ -472,8 +472,6 @@
   pref("javascript.options.wasm_gc",              false);
 //@line 1113 "$SRCDIR/modules/libpref/init/all.js"
   pref("javascript.options.wasm_multi_value",     true);
-//@line 1117 "$SRCDIR/modules/libpref/init/all.js"
-    pref("javascript.options.wasm_simd",            true);
 //@line 1122 "$SRCDIR/modules/libpref/init/all.js"
 pref("javascript.options.native_regexp",    true);
 pref("javascript.options.parallel_parsing", true);

Comment 10

[Lars T Hansen [:lth]]

Good point. That's bug 1669463. We're almost ready to land that, at which point we'll have feature parity and the difference should disappear. Going forward, we're committed to adding every new feature to every compiler at the same time.

Comment 11

[Mike Hommey [:glandium]]

Attachment: Bug 1675385 - Move architecture-dependent wasm prefs to static prefs.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a0fb6731557
Move architecture-dependent wasm prefs to static prefs. r=lth

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9a0fb6731557