Closed Bug 1638779 Opened 8 months ago Closed 8 months ago

CacheIRCompiler::emitMathAbsNumberResult has dubious float register uses

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: anba, Assigned: jandem)

References

Details

Attachments

(1 file)

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.

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

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Severity: -- → S3
Priority: -- → P2
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e45bf764fb12
Use AutoScratchFloatRegister more in emitMathAbsNumberResult. r=evilpie
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
You need to log in before you can comment on or make changes to this bug.