1981896 - 6.8% regression on Kraken-imaging-darkroom (Windows) and 8.5% on gaussian-blur (Linux) on 7Aug2025

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mayank Bansal]

See: https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&replicates=0&selected=5306405,2090567002&series=autoland,5306405,1,1&timerange=5184000&zoom=1754563273483,1754611757388,48.06364649512862,54.7498532610434

Suspect: bug 1981133 (backfills in progress)

Given the magnitude of improvements I dont think this regsression is serious. But filing for posterity.

Comment 1

[Mayank Bansal]

bisection confirmed bug 1981133 as the regressor.

Comment 2

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1981133

:jandem, since you are the author of the regressor, bug 1981133, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Jan de Mooij [:jandem]]

Very interesting. I can reproduce the regression locally. Our machine code looks more compact now, but we're slower. It seems related to this code:

function FastLog2(x) {
  return Math.log(x) / Math.LN2;
}

var LOG2_HALF = FastLog2(0.5);

function FastBias(b, x) {
  return Math.pow(x, FastLog2(b) / LOG2_HALF);
}

We inline FastLog2 into FastBias.

With my constant-global optimization, the JIT knows that LOG2_HALF is -1, so we can turn the Div instruction into a NegD instruction:

[MathD:Div]
vdivsd     %xmm1, %xmm0, %xmm1

==>

[NegD]
pcmpeqw    %xmm15, %xmm15
psllq      $63, %xmm15
xorpd      %xmm15, %xmm0

This is fine but it results in a small register allocation difference further down:

[PowD]
vmovapd    %xmm2, %xmm0
call       .Lfrom549

==>

[PowD]
xorpd      %xmm0, %xmm1
xorpd      %xmm1, %xmm0
xorpd      %xmm0, %xmm1
call       .Lfrom440

Apparently the CPU doesn't like this XOR-swap idiom. If I comment out that optimization we're faster locally, even though the swap now goes through memory instead of using the scratch double register..

Comment 4

[Mayank Bansal]

Linux,

• it is 6.5% improvement.
• BUT a 8.5% regression on gaussian-blur

OSX1470

• 10.4% regression on darkroom
• 3.7% improvement on desaturate

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1981896 - Don't use XOR-swap idiom for simple float register cycles. r?iain!

This fixes the imaging-darkroom regression in the bug and eliminates a third of all
resource-stalls according to perf. Baking in global constants is now an improvement
on that test.

Wikipedia also mentions that XOR-swap can be slower than plain moves on modern CPUs:

https://en.wikipedia.org/wiki/XOR_swap_algorithm#Reasons_for_avoidance_in_practice

Most 'optimized cycles' have a swapCount of 1 but other cases do show up
and it's pretty easy to handle them too. This avoids falling back to a memory
slot.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d457a2ad7b56
https://hg.mozilla.org/integration/autoland/rev/e02c6e10f828
Don't use XOR-swap idiom for simple float register cycles. r=iain

Comment 7

[Jan de Mooij [:jandem]]

I found another codegen problem with gaussian-blur. We're now generating extra code for MKeepAliveObject, to load the constant object into a register that then isn't used anywhere. This has a pretty large effect on # of cycles and getting rid of that makes the test > 15% faster locally...

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/e02c6e10f828

Comment 9

[Mayank Bansal]

(In reply to Pulsebot from comment #6)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d457a2ad7b56
https://hg.mozilla.org/integration/autoland/rev/e02c6e10f828
Don't use XOR-swap idiom for simple float register cycles. r=iain

Win: Lead to a 14% improvement on the regression, which is also a 8% improvement on the baseline.
OSX: 17.6% improvement on regerssion, which is also a 7% improvement on baseline
Win: 2.9% improvement in Jetstream3-gaussian-blur

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1981896 - Improve check for constants in AddKeepAliveInstructions. r?iain!

The check for constants failed for the data array in the gaussian-blur test
because it was wrapped by MGuardShape.

Also add a check for nursery objects because it's a similar case.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9763f5ef0a1a
https://hg.mozilla.org/integration/autoland/rev/2e49fc049de0
Improve check for constants in AddKeepAliveInstructions. r=iain

Comment 12

[Mayank Bansal]

(In reply to Pulsebot from comment #11)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9763f5ef0a1a
https://hg.mozilla.org/integration/autoland/rev/2e49fc049de0
Improve check for constants in AddKeepAliveInstructions. r=iain

18.1% improvement on regression numbers on gaussian-blur only on Linux, which is a 2.5% on pre-baseline numbers.

Comment 13

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/2e49fc049de0