commit f96ea6c650e6458422e4817f9b5cb94b1c409d64
parent 84e0e220af9c2dc8f3adf84ba6e00ffbd19296df
Author: André Bargull <andre.bargull@gmail.com>
Date: Wed, 5 Nov 2025 20:55:55 +0000
Bug 1997975 - Part 5: Support modulus with constant in arm64. r=spidermonkey-reviewers,iain
Part 3 enabled `LModMaskI`, which revealed it's actually computing incorrect
results. Instead of trying to fix `LModMaskI` simply enable optimisations for
all constant divisors.
Part 6 will remove `LModMaskI`.
Drive-by change:
- Rename `UDivConstantI` to `UDivConstant` to match `LUDiv` and x86 code.
Differential Revision: https://phabricator.services.mozilla.com/D271106
Diffstat:
5 files changed, 228 insertions(+), 72 deletions(-)
diff --git a/js/src/jit/LIROps.yaml b/js/src/jit/LIROps.yaml
@@ -4415,18 +4415,32 @@
numerator: WordSized
arguments:
denominator: int32_t
- num_temps: 1
mir_op: Div
-- name: UDivConstantI
+- name: UDivConstant
result_type: WordSized
operands:
numerator: WordSized
arguments:
- denominator: int32_t
- num_temps: 1
+ denominator: uint32_t
mir_op: Div
+- name: ModConstantI
+ result_type: WordSized
+ operands:
+ numerator: WordSized
+ arguments:
+ denominator: int32_t
+ mir_op: Mod
+
+- name: UModConstant
+ result_type: WordSized
+ operands:
+ numerator: WordSized
+ arguments:
+ denominator: uint32_t
+ mir_op: Mod
+
- name: ModMaskI
result_type: WordSized
operands:
diff --git a/js/src/jit/ReciprocalMulConstants.h b/js/src/jit/ReciprocalMulConstants.h
@@ -7,6 +7,8 @@
#ifndef jit_ReciprocalMulConstants_h
#define jit_ReciprocalMulConstants_h
+#include "mozilla/MathAlgorithms.h"
+
#include <stdint.h>
namespace js::jit {
@@ -15,8 +17,8 @@ struct ReciprocalMulConstants {
int64_t multiplier;
int32_t shiftAmount;
- static ReciprocalMulConstants computeSignedDivisionConstants(uint32_t d) {
- return computeDivisionConstants(d, 31);
+ static ReciprocalMulConstants computeSignedDivisionConstants(int32_t d) {
+ return computeDivisionConstants(mozilla::Abs(d), 31);
}
static ReciprocalMulConstants computeUnsignedDivisionConstants(uint32_t d) {
diff --git a/js/src/jit/arm64/CodeGenerator-arm64.cpp b/js/src/jit/arm64/CodeGenerator-arm64.cpp
@@ -477,35 +477,25 @@ void CodeGenerator::visitDivPowTwoI(LDivPowTwoI* ins) {
}
}
-void CodeGenerator::visitDivConstantI(LDivConstantI* ins) {
- const ARMRegister lhs32 = toWRegister(ins->numerator());
- const ARMRegister lhs64 = toXRegister(ins->numerator());
- const ARMRegister const32 = toWRegister(ins->temp0());
- const ARMRegister output32 = toWRegister(ins->output());
- const ARMRegister output64 = toXRegister(ins->output());
+template <class LDivOrMod>
+static void DivideWithConstant(MacroAssembler& masm, LDivOrMod* ins) {
+ ARMRegister lhs32 = toWRegister(ins->numerator());
+ ARMRegister lhs64 = toXRegister(ins->numerator());
+ ARMRegister output32 = toWRegister(ins->output());
+ ARMRegister output64 = toXRegister(ins->output());
int32_t d = ins->denominator();
- MDiv* mir = ins->mir();
+ vixl::UseScratchRegisterScope temps(&masm.asVIXL());
+ ARMRegister const32 = temps.AcquireW();
// The absolute value of the denominator isn't a power of 2.
- using mozilla::Abs;
- MOZ_ASSERT(!mozilla::IsPowerOfTwo(Abs(d)));
+ MOZ_ASSERT(!mozilla::IsPowerOfTwo(mozilla::Abs(d)));
- if (d == 0) {
- if (mir->trapOnError()) {
- masm.wasmTrap(wasm::Trap::IntegerDivideByZero, mir->trapSiteDesc());
- } else if (mir->canTruncateInfinities()) {
- masm.Mov(output32, wzr);
- } else {
- MOZ_ASSERT(mir->fallible());
- bailout(ins->snapshot());
- }
- return;
- }
+ auto* mir = ins->mir();
// We will first divide by Abs(d), and negate the answer if d is negative.
// If desired, this can be avoided by generalizing computeDivisionConstants.
- auto rmc = ReciprocalMulConstants::computeSignedDivisionConstants(Abs(d));
+ auto rmc = ReciprocalMulConstants::computeSignedDivisionConstants(d);
// We first compute (M * n) >> 32, where M = rmc.multiplier.
masm.Mov(const32, int32_t(rmc.multiplier));
@@ -542,21 +532,48 @@ void CodeGenerator::visitDivConstantI(LDivConstantI* ins) {
if (d < 0) {
masm.Neg(output32, output32);
}
+}
+
+void CodeGenerator::visitDivConstantI(LDivConstantI* ins) {
+ ARMRegister lhs32 = toWRegister(ins->numerator());
+ ARMRegister output32 = toWRegister(ins->output());
+ int32_t d = ins->denominator();
+
+ MDiv* mir = ins->mir();
+
+ if (d == 0) {
+ if (mir->trapOnError()) {
+ masm.wasmTrap(wasm::Trap::IntegerDivideByZero, mir->trapSiteDesc());
+ } else if (mir->canTruncateInfinities()) {
+ masm.Mov(output32, wzr);
+ } else {
+ MOZ_ASSERT(mir->fallible());
+ bailout(ins->snapshot());
+ }
+ return;
+ }
+
+ // Compute the truncated division result in output32.
+ DivideWithConstant(masm, ins);
if (!mir->isTruncated()) {
+ vixl::UseScratchRegisterScope temps(&masm.asVIXL());
+ ARMRegister temp32 = temps.AcquireW();
+ Register temp = temp32.asUnsized();
+
// This is a division op. Multiply the obtained value by d to check if
// the correct answer is an integer. This cannot overflow, since |d| > 1.
- masm.Mov(const32, d);
- masm.Msub(const32, output32, const32, lhs32);
+ masm.Mov(temp32, d);
+ masm.Msub(temp32, output32, temp32, lhs32);
if (d > 0) {
// bailout if (lhs - output * d != 0)
- bailoutTest32(Assembler::NonZero, const32, const32, ins->snapshot());
+ bailoutTest32(Assembler::NonZero, temp, temp, ins->snapshot());
} else {
MOZ_ASSERT(d < 0);
// bailout if (lhs - output * d != 0)
- masm.Cmp(const32, wzr);
+ masm.Cmp(temp32, wzr);
// If lhs is zero and the divisor is negative, the answer should have
// been -0.
@@ -573,32 +590,19 @@ void CodeGenerator::visitDivConstantI(LDivConstantI* ins) {
}
}
-void CodeGenerator::visitUDivConstantI(LUDivConstantI* ins) {
- const ARMRegister lhs32 = toWRegister(ins->numerator());
- const ARMRegister lhs64 = toXRegister(ins->numerator());
- const ARMRegister const32 = toWRegister(ins->temp0());
- const ARMRegister output32 = toWRegister(ins->output());
- const ARMRegister output64 = toXRegister(ins->output());
+template <class LUDivOrUMod>
+static void UnsignedDivideWithConstant(MacroAssembler& masm, LUDivOrUMod* ins) {
+ ARMRegister lhs32 = toWRegister(ins->numerator());
+ ARMRegister lhs64 = toXRegister(ins->numerator());
+ ARMRegister output64 = toXRegister(ins->output());
uint32_t d = ins->denominator();
- MDiv* mir = ins->mir();
+ vixl::UseScratchRegisterScope temps(&masm.asVIXL());
+ ARMRegister const32 = temps.AcquireW();
// The denominator isn't a power of 2 (see LDivPowTwoI).
MOZ_ASSERT(!mozilla::IsPowerOfTwo(d));
- if (d == 0) {
- if (ins->mir()->trapOnError()) {
- masm.wasmTrap(wasm::Trap::IntegerDivideByZero,
- ins->mir()->trapSiteDesc());
- } else if (mir->canTruncateInfinities()) {
- masm.Mov(output32, wzr);
- } else {
- MOZ_ASSERT(mir->fallible());
- bailout(ins->snapshot());
- }
- return;
- }
-
auto rmc = ReciprocalMulConstants::computeUnsignedDivisionConstants(d);
// We first compute (M * n), where M = rmc.multiplier.
@@ -627,16 +631,43 @@ void CodeGenerator::visitUDivConstantI(LUDivConstantI* ins) {
// (M * n) >> (32 + shift) is the truncated division answer.
masm.Lsr(output64, output64, 32 + rmc.shiftAmount);
}
+}
+
+void CodeGenerator::visitUDivConstant(LUDivConstant* ins) {
+ ARMRegister lhs32 = toWRegister(ins->numerator());
+ ARMRegister output32 = toWRegister(ins->output());
+ uint32_t d = ins->denominator();
+
+ MDiv* mir = ins->mir();
+
+ if (d == 0) {
+ if (ins->mir()->trapOnError()) {
+ masm.wasmTrap(wasm::Trap::IntegerDivideByZero, mir->trapSiteDesc());
+ } else if (mir->canTruncateInfinities()) {
+ masm.Mov(output32, wzr);
+ } else {
+ MOZ_ASSERT(mir->fallible());
+ bailout(ins->snapshot());
+ }
+ return;
+ }
+
+ // Compute the truncated division result in output32.
+ UnsignedDivideWithConstant(masm, ins);
// We now have the truncated division value. We are checking whether the
// division resulted in an integer, we multiply the obtained value by d and
// check the remainder of the division.
if (!mir->isTruncated()) {
- masm.Mov(const32, d);
- masm.Msub(const32, output32, const32, lhs32);
+ vixl::UseScratchRegisterScope temps(&masm.asVIXL());
+ ARMRegister temp32 = temps.AcquireW();
+ Register temp = temp32.asUnsized();
+
+ masm.Mov(temp32, d);
+ masm.Msub(temp32, output32, temp32, lhs32);
+
// bailout if (lhs - output * d != 0)
- bailoutTest32(Assembler::NonZero, const32.asUnsized(), const32.asUnsized(),
- ins->snapshot());
+ bailoutTest32(Assembler::NonZero, temp, temp, ins->snapshot());
}
}
@@ -730,6 +761,85 @@ void CodeGenerator::visitModPowTwoI(LModPowTwoI* ins) {
}
}
+void CodeGenerator::visitModConstantI(LModConstantI* ins) {
+ Register lhs = ToRegister(ins->numerator());
+ ARMRegister lhs32 = toWRegister(ins->numerator());
+ ARMRegister output32 = toWRegister(ins->output());
+
+ MMod* mir = ins->mir();
+
+ int32_t d = ins->denominator();
+ if (d == 0) {
+ if (mir->trapOnError()) {
+ masm.wasmTrap(wasm::Trap::IntegerDivideByZero, mir->trapSiteDesc());
+ } else if (mir->isTruncated()) {
+ masm.Mov(output32, wzr);
+ } else {
+ MOZ_ASSERT(mir->fallible());
+ bailout(ins->snapshot());
+ }
+ return;
+ }
+
+ // Compute the truncated division result in output32.
+ DivideWithConstant(masm, ins);
+
+ // Compute the remainder: output = lhs - (output * rhs).
+ {
+ vixl::UseScratchRegisterScope temps(&masm.asVIXL());
+ ARMRegister rhs32 = temps.AcquireW();
+
+ masm.Mov(rhs32, d);
+ masm.Msub(output32, output32, rhs32, lhs32);
+ }
+
+ if (mir->canBeNegativeDividend() && !mir->isTruncated()) {
+ // If output == 0 and lhs < 0, then the result should be double -0.0.
+ Label done;
+ masm.Cbnz(output32, &done);
+ bailoutCmp32(Assembler::LessThan, lhs, Imm32(0), ins->snapshot());
+ masm.bind(&done);
+ }
+}
+
+void CodeGenerator::visitUModConstant(LUModConstant* ins) {
+ Register output = ToRegister(ins->output());
+ ARMRegister lhs32 = toWRegister(ins->numerator());
+ ARMRegister output32 = toWRegister(ins->output());
+
+ MMod* mir = ins->mir();
+
+ uint32_t d = ins->denominator();
+ if (d == 0) {
+ if (ins->mir()->trapOnError()) {
+ masm.wasmTrap(wasm::Trap::IntegerDivideByZero, mir->trapSiteDesc());
+ } else if (mir->isTruncated()) {
+ masm.Mov(output32, wzr);
+ } else {
+ MOZ_ASSERT(mir->fallible());
+ bailout(ins->snapshot());
+ }
+ return;
+ }
+
+ // Compute the truncated division result in output32.
+ UnsignedDivideWithConstant(masm, ins);
+
+ // Compute the remainder: output = lhs - (output * rhs).
+ {
+ vixl::UseScratchRegisterScope temps(&masm.asVIXL());
+ ARMRegister rhs32 = temps.AcquireW();
+
+ masm.Mov(rhs32, d);
+ masm.Msub(output32, output32, rhs32, lhs32);
+ }
+
+ // Bail if not truncated and the remainder is in the range [2^31, 2^32).
+ if (!ins->mir()->isTruncated()) {
+ bailoutTest32(Assembler::Signed, output, output, ins->snapshot());
+ }
+}
+
void CodeGenerator::visitModMaskI(LModMaskI* ins) {
MMod* mir = ins->mir();
int32_t shift = ins->shift();
diff --git a/js/src/jit/arm64/Lowering-arm64.cpp b/js/src/jit/arm64/Lowering-arm64.cpp
@@ -19,8 +19,6 @@
using namespace js;
using namespace js::jit;
-using mozilla::FloorLog2;
-
LBoxAllocation LIRGeneratorARM64::useBoxFixed(MDefinition* mir, Register reg1,
Register, bool useAtStart) {
MOZ_ASSERT(mir->type() == MIRType::Value);
@@ -219,7 +217,7 @@ void LIRGeneratorARM64::lowerDivI(MDiv* div) {
return;
}
- auto* lir = new (alloc()) LDivConstantI(lhs, temp(), rhs);
+ auto* lir = new (alloc()) LDivConstantI(lhs, rhs);
if (div->fallible()) {
assignSnapshot(lir, div->bailoutKind());
}
@@ -265,30 +263,39 @@ void LIRGeneratorARM64::lowerMulI(MMul* mul, MDefinition* lhs,
}
void LIRGeneratorARM64::lowerModI(MMod* mod) {
+ LAllocation lhs = useRegister(mod->lhs());
+
if (mod->rhs()->isConstant()) {
int32_t rhs = mod->rhs()->toConstant()->toInt32();
- int32_t shift = FloorLog2(rhs);
- if (rhs > 0 && 1 << shift == rhs) {
- LModPowTwoI* lir =
- new (alloc()) LModPowTwoI(useRegister(mod->lhs()), shift);
+ int32_t shift = mozilla::FloorLog2(mozilla::Abs(rhs));
+
+ if (rhs != 0 && uint32_t(1) << shift == mozilla::Abs(rhs)) {
+ auto* lir = new (alloc()) LModPowTwoI(lhs, shift);
if (mod->fallible()) {
assignSnapshot(lir, mod->bailoutKind());
}
define(lir, mod);
return;
- } else if (shift < 31 && (1 << (shift + 1)) - 1 == rhs) {
- LModMaskI* lir = new (alloc())
- LModMaskI(useRegister(mod->lhs()), temp(), temp(), shift + 1);
+ }
+
+ if (shift < 31 && (1 << (shift + 1)) - 1 == rhs) {
+ auto* lir = new (alloc()) LModMaskI(lhs, temp(), temp(), shift + 1);
if (mod->fallible()) {
assignSnapshot(lir, mod->bailoutKind());
}
define(lir, mod);
return;
}
+
+ auto* lir = new (alloc()) LModConstantI(lhs, rhs);
+ if (mod->fallible()) {
+ assignSnapshot(lir, mod->bailoutKind());
+ }
+ define(lir, mod);
+ return;
}
- auto* lir =
- new (alloc()) LModI(useRegister(mod->lhs()), useRegister(mod->rhs()));
+ auto* lir = new (alloc()) LModI(lhs, useRegister(mod->rhs()));
if (mod->fallible()) {
assignSnapshot(lir, mod->bailoutKind());
}
@@ -511,7 +518,7 @@ void LIRGeneratorARM64::lowerUDiv(MDiv* div) {
return;
}
- auto* lir = new (alloc()) LUDivConstantI(lhs, temp(), rhs);
+ auto* lir = new (alloc()) LUDivConstant(lhs, rhs);
if (div->fallible()) {
assignSnapshot(lir, div->bailoutKind());
}
@@ -537,8 +544,31 @@ void LIRGeneratorARM64::lowerUDiv(MDiv* div) {
}
void LIRGeneratorARM64::lowerUMod(MMod* mod) {
- auto* lir =
- new (alloc()) LUMod(useRegister(mod->lhs()), useRegister(mod->rhs()));
+ LAllocation lhs = useRegister(mod->lhs());
+
+ if (mod->rhs()->isConstant()) {
+ // NOTE: the result of toInt32 is coerced to uint32_t.
+ uint32_t rhs = mod->rhs()->toConstant()->toInt32();
+ int32_t shift = mozilla::FloorLog2(rhs);
+
+ if (rhs != 0 && uint32_t(1) << shift == rhs) {
+ auto* lir = new (alloc()) LModPowTwoI(lhs, shift);
+ if (mod->fallible()) {
+ assignSnapshot(lir, mod->bailoutKind());
+ }
+ define(lir, mod);
+ return;
+ }
+
+ auto* lir = new (alloc()) LUModConstant(lhs, rhs);
+ if (mod->fallible()) {
+ assignSnapshot(lir, mod->bailoutKind());
+ }
+ define(lir, mod);
+ return;
+ }
+
+ auto* lir = new (alloc()) LUMod(lhs, useRegister(mod->rhs()));
if (mod->fallible()) {
assignSnapshot(lir, mod->bailoutKind());
}
diff --git a/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp b/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp
@@ -1103,11 +1103,11 @@ void CodeGenerator::visitDivOrModConstantI(LDivOrModConstantI* ins) {
// The absolute value of the denominator isn't a power of 2 (see LDivPowTwoI
// and LModPowTwoI).
- MOZ_ASSERT((Abs(d) & (Abs(d) - 1)) != 0);
+ MOZ_ASSERT(!mozilla::IsPowerOfTwo(mozilla::Abs(d)));
// We will first divide by Abs(d), and negate the answer if d is negative.
// If desired, this can be avoided by generalizing computeDivisionConstants.
- auto rmc = ReciprocalMulConstants::computeSignedDivisionConstants(Abs(d));
+ auto rmc = ReciprocalMulConstants::computeSignedDivisionConstants(d);
// We first compute (M * n) >> 32, where M = rmc.multiplier.
masm.movl(Imm32(rmc.multiplier), eax);
