Closed Bug 2087950 Opened 13 days ago Closed 4 days ago

Make rbx the x64 JIT scratch register to shrink JIT code

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
162 Branch
Tracking Status
firefox161 --- affected
firefox162 --- fixed

People

(Reporter: andrew, Assigned: andrew)

Details

Attachments

(1 external file)

On x64, ScratchReg is r11, so every scratch use that is not already a 64-bit operation pays a REX prefix byte for the register number alone. The scratch register is among the most used registers in Baseline and Ion code.

rbx is the only low register without a fixed role in the JIT ABI, apart from being Baseline's R1. Swapping the two makes ScratchReg rbx and R1 r11. CallTempReg2, ABINonArgReg1, WasmJitEntryReturnScratch, and the temporaries hard-coded as rbx in the trampolines, flexibleDivMod64 and the wasm catch-handler path move to r11 as well. rbx becomes non-allocatable in place of r11.

Measured on JetStream 3 in the x86-64 shell from a dump of every JIT code blob, per instruction to factor out run-to-run differences in what gets compiled:

  • Baseline: 0.80% smaller (1.01% in total bytes, 109.1 MB -> 108.0 MB)
  • Ion: 0.6-1.1% smaller
  • WasmBaseline: 1.06% smaller
  • WasmIon: 0.29% larger (--no-threads, 85.3 MB), since rbx is no longer allocatable there
  • Trampolines: 1-2% larger, since R1 now needs REX

Overall about 0.86% of all JIT code bytes.

JetStream 3 scores show no measurable speed change: over 8 paired, interleaved rounds against an unpatched shell, the overall score moved -0.55% (95% CI -1.26% to +0.16%), JS tests -0.68% +/- 0.77%, wasm tests +0.10% +/- 1.34%. The case for the change is code size, not speed.

An alternative use of rbx is to hold the wasm heap base (HeapReg, currently r15), which shrinks WasmBaseline code by 1.26% and WasmIon by 1.19% but leaves JS tiers unchanged. The two are mutually exclusive, since both need rbx.

A patch passes jit-tests on Linux x64 debug and opt, and on Windows x64 try: https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=112976

Found via x86lint, whose register census counts the REX and VEX bytes paid only because of the register number.

ScratchReg is r11, so every scratch use that is not already 64-bit pays a REX
byte for the register number alone. It is among the most used registers in
Baseline and Ion code. rbx is the only low register without a fixed role in
the JIT ABI, apart from being Baseline's R1, so swap the two: ScratchReg
becomes rbx, and R1, CallTempReg2, ABINonArgReg1, WasmJitEntryReturnScratch
and the temporaries that the trampolines, flexibleDivMod64 and the wasm
catch-handler path hard-code as rbx move to r11.

Measured on JetStream 3 in the x86-64 shell from a dump of every JIT code
blob, per instruction to factor out run-to-run differences in what gets
compiled: Baseline code is 0.80% smaller (1.01% in total bytes, 109.1 MB ->
108.0 MB), Ion code 0.6-1.1% smaller and WasmBaseline code 1.06% smaller.
WasmIon code grows 0.29% (--no-threads, 85.3 MB) now that rbx is no longer
allocatable there, and the trampolines grow 1-2% because R1 now needs REX.

The codegen tests that name the scratch register now expect rbx. With rbx no
longer allocatable in wasm code, Ion drops the extra move in the Linux
select test and uses rsi where the Windows-only select test expected rbx.

Found via x86lint.

Assignee: nobody → andrew
Status: NEW → ASSIGNED
Pushed by agoloman@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/5a3f6397e615 https://hg.mozilla.org/integration/autoland/rev/098aacc52227 Revert "Bug 2087950 - Make rbx the x64 JIT scratch register and move R1 to r11. r=jandem" for causing windows mass failures.

Backed out for causing windows mass failures.

Flags: needinfo?(andrew)

The backout in comment 4 was caused by regexp code not saving rbx.

SavedNonVolatileRegisters, which decides which callee-saved registers the
regexp code saves on entry, starts from GeneralRegisterSet::NonVolatile().
That set only includes allocatable registers, so rbx dropped out once it
became the non-allocatable ScratchReg. Regexp code is called directly from
C++ and uses ScratchReg for absolute-address accesses, so it clobbered rbx,
which is callee-saved on both System V and Win64. Any C++ caller keeping a
value in rbx across the call was corrupted: the SEGV in
js::irregexp::Execute in the Linux log, and most likely the xpcshell server
that never started on Windows.

jit-tests did not catch it because it depends on whether the compiler keeps
something in rbx across that call, which the debug shell build did not.

The updated patch adds rbx to SavedNonVolatileRegisters on x64. The other
places where C++ calls into JIT code already save rbx: the JIT entry
trampoline (generateEnterJIT) explicitly, and the wasm entry stubs through
Registers::NonVolatileMask.

ASan mochitests on Windows and Linux are green, including the
mochitest-plain and webgl2-ext jobs that failed:
https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=117823

Flags: needinfo?(andrew)
Status: ASSIGNED → RESOLVED
Closed: 4 days ago
Resolution: --- → FIXED
Target Milestone: --- → 162 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: