1638779 - CacheIRCompiler::emitMathAbsNumberResult has dubious float register uses

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[André Bargull [:anba]]

CacheIRCompiler::emitMathAbsNumberResult calls ensureDoubleRegister with FloatReg0 as the destination register. This will irrevocably clobber FloatReg0, because "CacheIR's register allocator doesn't work with FP registers" (per https://phabricator.services.mozilla.com/D47757#inline-292972, also see bug 1586404).

Later on, AutoScratchFloatRegister is used. AutoScratchFloatRegister always uses FloatReg0 as the scratch register. This gives the impression that FloatReg0 has to be recovered for some reason, but as mentioned above, FloatReg0 is already overwritten.

MathAbsNumberResult should probably be marked as non-shared in CacheIROps.yaml and then only implemented for Baseline (, because FloatReg0 can always be used for Baseline CacheIR and doesn't need to be recovered). Furthermore the code can be simplified by removing AutoScratchFloatRegister.

Comment 1

[Jan de Mooij [:jandem]]

I just noticed this too when I was looking into implementing MathSqrt and changed it to use scratch instead of FloatReg0.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1638779 - Use AutoScratchFloatRegister more in emitMathAbsNumberResult. r?evilpie!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e45bf764fb12
Use AutoScratchFloatRegister more in emitMathAbsNumberResult. r=evilpie

Comment 4

[Alexandru Michis [:malexandru]]

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