commit dddc9fc62a56f7286db6d09c89078bbf0c5dea90
parent af7592e9e83b84cf5af6555c246d62c562817088
Author: Ryan Hunt <rhunt@eqrion.net>
Date: Fri, 12 Dec 2025 20:50:00 +0000
Bug 1992240 - wasm: Fix JIT entry when negative denormal is used without denormal support. r=jandem
Add a canonicalizeDoubleZero which flushes denormals to zero using an
identity instruction. The JIT entry eagerly will flush double values
to zero with this, so that the `branchValueConvertsToWasmAnyRefInline`
and `mozilla::NumberIsInt32` methods behave consistently even with
denormals disabled.
Differential Revision: https://phabricator.services.mozilla.com/D275283
Diffstat:
4 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/js/src/jit-test/tests/auto-regress/bug1940716.js b/js/src/jit-test/tests/auto-regress/bug1940716.js
@@ -1,4 +1,4 @@
-// |jit-test| --disable-main-thread-denormals; skip-if: !getBuildConfiguration("can-disable-main-thread-denormals") || !wasmIsSupported() || (getBuildConfiguration("osx") && getBuildConfiguration("arm64"));
+// |jit-test| --disable-main-thread-denormals; skip-if: !getBuildConfiguration("can-disable-main-thread-denormals") || !wasmIsSupported();
function a(b) {
c = new WebAssembly.Module(b);
@@ -7,7 +7,9 @@ function a(b) {
function d(e) {
return a(wasmTextToBinary(e));
}
-f = [ , Number.MIN_VALUE ]
let { refTest } = d(`(func (export "refTest") (param externref))`).exports;
+
+// Ensure there are enough values to trigger a wasm JIT entry
+f = Array(16).fill([Number.MIN_VALUE, -Number.MIN_VALUE]).flat();
for (h of f)
refTest(h);
diff --git a/js/src/jit/MacroAssembler-inl.h b/js/src/jit/MacroAssembler-inl.h
@@ -883,6 +883,20 @@ void MacroAssembler::canonicalizeDoubleNaN(FloatRegister reg) {
bind(¬NaN);
}
+void MacroAssembler::canonicalizeDoubleZero(FloatRegister reg,
+ FloatRegister scratch) {
+ // If denormals are disabled, then operations on them will flush denormal
+ // values to zero (FTZ flag). We need the cheapest operation that is the
+ // identity function.
+ //
+ // Unfortunately, just moving the float register doesn't trigger FTZ. Adding
+ // '+-0' isn't an identity function, because it can toggle the sign bit.
+ //
+ // Therefore we choose to multiply by 1.0, which won't change the result.
+ loadConstantDouble(1.0, scratch);
+ mulDouble(scratch, reg);
+}
+
// ========================================================================
// Memory access primitives.
diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h
@@ -2233,6 +2233,14 @@ class MacroAssembler : public MacroAssemblerSpecific {
inline void canonicalizeFloatNaN(FloatRegister reg);
+ // If denormal support is disabled, there are 2^53 ways to represent zero.
+ // This function canonicalizes the representation to either -0.0 or +0.0,
+ // maintaining the sign bit of the input.
+ //
+ // This function will not change the value of the double if denormals are
+ // enabled.
+ inline void canonicalizeDoubleZero(FloatRegister reg, FloatRegister scratch);
+
public:
// ========================================================================
// Memory access primitives.
diff --git a/js/src/wasm/WasmStubs.cpp b/js/src/wasm/WasmStubs.cpp
@@ -1111,6 +1111,37 @@ static bool GenerateJitEntry(MacroAssembler& masm, size_t funcExportIndex,
case ValType::Ref: {
// Guarded against by temporarilyUnsupportedReftypeForEntry()
MOZ_RELEASE_ASSERT(funcType.args()[i].refType().isExtern());
+
+ // If the value is a double that is a negative denormal and denormals
+ // are disabled, then `branchValueConvertsToWasmAnyRefInline` will view
+ // it as '-0' (which must be boxed in the OOL path). However, the
+ // AnyRef boxing code uses `mozilla::NumberIsInt32` which does not
+ // properly handle the CPU DAZ/FTZ flags and returns true and therefore
+ // doesn't box the double. This leads to an assertion below where we
+ // expected the value to be boxed.
+ //
+ // Making `mozilla::NumberIsInt32` handle the CPU DAZ/FTZ flags would
+ // add a significant cost to many hot-paths. We instead just
+ // eagerly canonicalize denormals to +-0.0 here to avoid inconsistent
+ // results (see Bug 1971519).
+ {
+ ScratchDoubleScope tmpD(masm);
+
+ Label notDouble;
+ masm.branchTestDouble(Assembler::NotEqual, scratchV, ¬Double);
+
+ // Unbox the double and canonicalize any denormal values
+ masm.unboxDouble(scratchV, scratchF);
+ masm.canonicalizeDoubleZero(scratchF, tmpD);
+
+ // Box the value back into scratchV and also store it back to the
+ // stack.
+ masm.boxDouble(scratchF, scratchV, tmpD);
+ masm.storeValue(scratchV, jitArgAddr);
+
+ masm.bind(¬Double);
+ }
+
masm.branchValueConvertsToWasmAnyRefInline(scratchV, scratchG, scratchF,
&next);
masm.jump(&oolCall);