From 065c095849ccee08453081a41373f0ecbe1dc6d2 Mon Sep 17 00:00:00 2001 From: Lars T Hansen Date: Mon, 15 Mar 2021 16:44:30 +0000 Subject: [PATCH] Bug 1678097 - Enable Ion for wasm on arm64 without SIMD (phase 0). r=lth. In Phase 0, both Cranelift and Ion are available on arm64, and Ion is the default. Use --wasm-force-cranelift or --wasm-compiler=cranelift at the shell to select Cranelift, or set javascript.options.wasm_force_ion to false in about:config. Phase 0 is appropriate for developers, before the patch set lands in mozilla-central and before SIMD is present. In Phase 1, both compilers are still available, but the default is switched to Cranelift. Use --wasm-force-ion or --wasm-compiler=ion at the shell to select Ion, or make sure javascript.options.wasm_force_ion is true in about:config. Phase 1 is appropriate for fuzzing, after the patch set lands in mozilla-central but before Ion is enabled by default. The patch for Phase 1 will appear on bug 1678097 and will be very small, and MUST land with the patch for Phase 0. In Phase 0 and Phase 1, --wasm-compiler=cranelift and --wasm-compiler=ion are both accepted, and do the expected thing. In Phase 2, Cranelift becomes disabled in moz.configure and all the changes in the present patch are removed again. The patch for Phase 2 will appear on bug 1686626 and will revert Phase 0 and Phase 1, and additionally update moz.configure. Differential Revision: https://phabricator.services.mozilla.com/D102420 --- js/src/builtin/TestingFunctions.cpp | 7 ++++ js/src/fuzz-tests/testWasm.cpp | 9 +++- js/src/shell/js.cpp | 53 +++++++++++++++++++++--- js/src/shell/jsshell.h | 4 ++ js/src/wasm/WasmIonCompile.cpp | 2 +- js/xpconnect/src/XPCJSContext.cpp | 10 ++++- modules/libpref/init/StaticPrefList.yaml | 12 +++++- 7 files changed, 88 insertions(+), 9 deletions(-) diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index 216ed6ad1111..7fd78729ffc5 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -899,6 +899,13 @@ static bool WasmCompilersPresent(JSContext* cx, unsigned argc, Value* vp) { } strcat(buf, "cranelift"); } + // Cranelift->Ion transition + if (wasm::IonPlatformSupport()) { + if (*buf) { + strcat(buf, ","); + } + strcat(buf, "ion"); + } #else if (wasm::IonPlatformSupport()) { if (*buf) { diff --git a/js/src/fuzz-tests/testWasm.cpp b/js/src/fuzz-tests/testWasm.cpp index 8a2024546c86..8321f5187533 100644 --- a/js/src/fuzz-tests/testWasm.cpp +++ b/js/src/fuzz-tests/testWasm.cpp @@ -144,7 +144,13 @@ static int testWasmFuzz(const uint8_t* buf, size_t size) { // that may have been enabled. bool enableWasmBaseline = ((optByte & 0xF0) == (1 << 7)); bool enableWasmOptimizing = false; +#ifdef JS_CODEGEN_ARM64 + // Cranelift->Ion transition + bool forceWasmIon = false; +#endif #ifdef ENABLE_WASM_CRANELIFT + // Cranelift->Ion transition + forceWasmIon = IonPlatformSupport() && ((optByte & 0xF0) == (1 << 6)); enableWasmOptimizing = CraneliftPlatformSupport() && ((optByte & 0xF0) == (1 << 5)); #else @@ -182,7 +188,8 @@ static int testWasmFuzz(const uint8_t* buf, size_t size) { JS::ContextOptionsRef(gCx) .setWasmBaseline(enableWasmBaseline) #ifdef ENABLE_WASM_CRANELIFT - .setWasmCranelift(enableWasmOptimizing) + .setWasmCranelift(enableWasmOptimizing && !forceWasmIon) + .setWasmIon(forceWasmIon) #else .setWasmIon(enableWasmOptimizing) #endif diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 63eba7472616..55ac234cfd22 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -591,6 +591,13 @@ bool shell::enableWasm = false; bool shell::enableSharedMemory = SHARED_MEMORY_DEFAULT; bool shell::enableWasmBaseline = false; bool shell::enableWasmOptimizing = false; +#ifdef JS_CODEGEN_ARM64 +// Cranelift->Ion transition. The right value for development is 'true'; when +// we land for phase 1, we flip this to 'false'; when we land for phase 2, we +// remove this flag. Also see the reading of the flag wasm-force-cranelift +// below; that becomes wasm-force-ion for phase 1. +bool shell::forceWasmIon = true; +#endif bool shell::enableWasmReftypes = true; #ifdef ENABLE_WASM_FUNCTION_REFERENCES bool shell::enableWasmFunctionReferences = false; @@ -10856,6 +10863,9 @@ static bool SetContextOptions(JSContext* cx, const OptionParser& op) { enableWasmBaseline = true; enableWasmOptimizing = true; + bool commandLineRequestedWasmIon = false; + bool commandLineRequestedWasmCranelift = false; + if (const char* str = op.getStringOption("wasm-compiler")) { if (strcmp(str, "none") == 0) { enableWasm = false; @@ -10874,17 +10884,20 @@ static bool SetContextOptions(JSContext* cx, const OptionParser& op) { } else if (strcmp(str, "cranelift") == 0) { enableWasmBaseline = false; enableWasmOptimizing = true; + commandLineRequestedWasmCranelift = true; } else if (strcmp(str, "baseline+cranelift") == 0) { MOZ_ASSERT(enableWasmBaseline); enableWasmOptimizing = true; -#else + commandLineRequestedWasmCranelift = true; +#endif } else if (strcmp(str, "ion") == 0) { enableWasmBaseline = false; enableWasmOptimizing = true; + commandLineRequestedWasmIon = true; } else if (strcmp(str, "baseline+ion") == 0) { MOZ_ASSERT(enableWasmBaseline); enableWasmOptimizing = true; -#endif + commandLineRequestedWasmIon = true; } else { return OptionFailure("wasm-compiler", str); } @@ -10931,13 +10944,35 @@ static bool SetContextOptions(JSContext* cx, const OptionParser& op) { enableTopLevelAwait = op.getBoolOption("enable-top-level-await"); useOffThreadParseGlobal = op.getBoolOption("off-thread-parse-global"); + // ifdeffing the defs and uses of these two so as to avoid unused-variable + // complaints on all targets is difficult. Simpler just to create fake uses. + mozilla::Unused << commandLineRequestedWasmIon; + mozilla::Unused << commandLineRequestedWasmCranelift; +#ifdef JS_CODEGEN_ARM64 + // Cranelift->Ion transition. When we land for phase 1, this becomes + // wasm-force-ion (be sure to update below around line 11500), and the default + // value of the flag is flipped to false, see comments above. When we land + // for phase 2, this goes away. + MOZ_ASSERT( + !(commandLineRequestedWasmIon && commandLineRequestedWasmCranelift)); + if (commandLineRequestedWasmIon) { + forceWasmIon = true; + } else if (commandLineRequestedWasmCranelift) { + forceWasmIon = false; + } else { + forceWasmIon = !op.getBoolOption("wasm-force-cranelift"); + } +#endif + JS::ContextOptionsRef(cx) .setAsmJS(enableAsmJS) .setWasm(enableWasm) .setWasmForTrustedPrinciples(enableWasm) .setWasmBaseline(enableWasmBaseline) #ifdef ENABLE_WASM_CRANELIFT - .setWasmCranelift(enableWasmOptimizing) + // Cranelift->Ion transition + .setWasmCranelift(enableWasmOptimizing && !forceWasmIon) + .setWasmIon(enableWasmOptimizing && forceWasmIon) #else .setWasmIon(enableWasmOptimizing) #endif @@ -11343,7 +11378,9 @@ static void SetWorkerContextOptions(JSContext* cx) { .setWasm(enableWasm) .setWasmBaseline(enableWasmBaseline) #ifdef ENABLE_WASM_CRANELIFT - .setWasmCranelift(enableWasmOptimizing) + // Cranelift->Ion transition + .setWasmCranelift(enableWasmOptimizing && !forceWasmIon) + .setWasmIon(enableWasmOptimizing && forceWasmIon) #else .setWasmIon(enableWasmOptimizing) #endif @@ -11896,7 +11933,13 @@ int main(int argc, char** argv, char** envp) { #else !op.addBoolOption('\0', "wasm-exceptions", "No-op") || #endif - +#ifdef JS_CODEGEN_ARM64 + // Cranelift->Ion transition. This becomes wasm-force-ion at Phase 1 of + // the landing and then disappears at Phase 2. See sundry comments above. + !op.addBoolOption( + '\0', "wasm-force-cranelift", + "Temporary: Force Cranelift in builds with both Cranelift and Ion") || +#endif !op.addBoolOption('\0', "no-native-regexp", "Disable native regexp compilation") || !op.addIntOption( diff --git a/js/src/shell/jsshell.h b/js/src/shell/jsshell.h index 8ce8ea4b80fe..2a9a1308a0f0 100644 --- a/js/src/shell/jsshell.h +++ b/js/src/shell/jsshell.h @@ -112,6 +112,10 @@ extern bool enableWasm; extern bool enableSharedMemory; extern bool enableWasmBaseline; extern bool enableWasmOptimizing; +#ifdef JS_CODEGEN_ARM64 +// Cranelift->Ion transition +extern bool forceWasmIon; +#endif extern bool enableWasmReftypes; #ifdef ENABLE_WASM_FUNCTION_REFERENCES extern bool enableWasmFunctionReferences; diff --git a/js/src/wasm/WasmIonCompile.cpp b/js/src/wasm/WasmIonCompile.cpp index 9332d0285605..aaa150f1cddf 100644 --- a/js/src/wasm/WasmIonCompile.cpp +++ b/js/src/wasm/WasmIonCompile.cpp @@ -5673,7 +5673,7 @@ bool wasm::IonCompileFunctions(const ModuleEnvironment& moduleEnv, bool js::wasm::IonPlatformSupport() { #if defined(JS_CODEGEN_X64) || defined(JS_CODEGEN_X86) || \ defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS32) || \ - defined(JS_CODEGEN_MIPS64) + defined(JS_CODEGEN_MIPS64) || defined(JS_CODEGEN_ARM64) return true; #else return false; diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index fee37f597f8a..a2554a1b3900 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -937,6 +937,12 @@ static void ReloadPrefsCallback(const char* pref, void* aXpccx) { Preferences::GetBool(JS_OPTIONS_DOT_STR "wasm_trustedprincipals"); bool useWasmOptimizing = Preferences::GetBool(JS_OPTIONS_DOT_STR "wasm_optimizingjit"); +#ifdef JS_CODEGEN_ARM64 + // Cranelift->Ion transition. When we land for phase 1, the default value in + // the prefs file changes from 'true' to 'false'. When we land for phase 2, + // this goes away. + bool forceWasmIon = Preferences::GetBool(JS_OPTIONS_DOT_STR "wasm_force_ion"); +#endif bool useWasmBaseline = Preferences::GetBool(JS_OPTIONS_DOT_STR "wasm_baselinejit"); bool useWasmReftypes = @@ -1035,7 +1041,9 @@ static void ReloadPrefsCallback(const char* pref, void* aXpccx) { .setWasm(useWasm) .setWasmForTrustedPrinciples(useWasmTrustedPrincipals) #ifdef ENABLE_WASM_CRANELIFT - .setWasmCranelift(useWasmOptimizing) + // Cranelift->Ion transition + .setWasmCranelift(useWasmOptimizing && !forceWasmIon) + .setWasmIon(useWasmOptimizing && forceWasmIon) #else .setWasmIon(useWasmOptimizing) #endif diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 7ddcd90ee29d..0e97840493b6 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -5440,7 +5440,7 @@ - name: javascript.options.wasm_optimizingjit type: bool -#if defined(MOZ_AARCH64) && !defined(ENABLE_WASM_CRANELIFT) +#if defined(MOZ_AARCH64) && !defined(NIGHTLY_BUILD) value: false #else value: true @@ -5458,6 +5458,16 @@ mirror: always #endif +#if defined(MOZ_AARCH64) +# Cranelift->Ion transition. Temporary switch, the value 'true' is correct for +# SM developers. When we land Phase 1 of the transition, the value of this will +# change to 'false'. When we land Phase 2, this switch will disappear altogether. +- name: javascript.options.wasm_force_ion + type: bool + value: true + mirror: always +#endif + #if defined(ENABLE_WASM_SIMD_WORMHOLE) && defined(EARLY_BETA_OR_EARLIER) # This is x64-only and it would break unified macOS builds if # it were in all.js. The feature rides the trains but not the