1770366 - Use frame pointer register in Warp/Ion

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P3)

Description

[Jan de Mooij [:jandem]]

After bug 1426134 it should become feasible to investigate this. This would involve adding the standard frame pointer prologue/epilogue code and relying on FP more instead of static stack depths.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1770366 part 1 - Rename *SlotCount to *SlotsSize in a few places. r?iain!

It used to be that slots were pointer-sized, but this was changed in bug 950703.
Rename "SlotCount" to "SlotsSize" to make clear that it's number of bytes.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1770366 part 2 - Don't include space for argument slots in safepoints. r?iain!

The only slots stored in the BitSet are local slots, so there's no need to include
the space we reserve for arguments pushed for callee functions. This should make
safepoints a bit more compact too.

This was the behavior before the fix in bug 771398. Other parts of that patch were
then reverted in bug 934502, after bug 952992 simplified how call arguments are
handled, but we never reverted this part.

Also removes StackOffsetToSlot, it was similarly unused.

Depends on D148641

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1770366 part 3 - Use unsigned integers for some offsets. r?iain!

Depends on D148642

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1770366 part 4 - Tidy up frame offset and size calculations. r?iain!

This adds offsetOfLocalSlots_ and offsetOfPassedArgSlots_, so that we can
more easily add the frame pointer to the frame in the next patch.

Depends on D148643

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1770366 part 5 - Add frame pointer to IonJS/Warp frames. r?iain!

Change OSR to use the standard frame epilogue on the Baseline side and frame prologue
on the CodeGenerator side.

Bailouts now have to copy the saved-frame-pointer from the Ion frame. This lets us
simplify the code too.

Depends on D148644

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1770366 part 6 - Rely on preserved frame pointer more after JIT ABI calls. r?iain!

Simplify and optimize Baseline stub frames, rectifier frames and Baseline generator-resume
to use the frame pointer register instead of computing it from the descriptor.

Depends on D148645

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df8aad2f05fa
part 1 - Rename *SlotCount to *SlotsSize in a few places. r=iain
https://hg.mozilla.org/integration/autoland/rev/83801e33f21a
part 2 - Don't include space for argument slots in safepoints. r=iain
https://hg.mozilla.org/integration/autoland/rev/789cf763c617
part 3 - Use unsigned integers for some offsets. r=iain
https://hg.mozilla.org/integration/autoland/rev/e8f787c59185
part 4 - Tidy up frame offset and size calculations. r=iain
https://hg.mozilla.org/integration/autoland/rev/1c35c1a7b373
part 5 - Add frame pointer to IonJS/Warp frames. r=iain
https://hg.mozilla.org/integration/autoland/rev/056b37d052bc
part 6 - Rely on preserved frame pointer more after JIT ABI calls. r=iain

Comment 8

[Atila Butkovits]

Backed out for causing failure at mouse-pointer-boundary-events-for-shadowdom.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/a087dace973356bec9cb9693016dffb58065a3a5

Failure log: https://treeherder.mozilla.org/logviewer?job_id=380769352&repo=autoland&lineNumber=15189

Push where failures started: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=e9jae1_LRCKAfQxZ8yldgg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=e2618316efab559914f12a1688bb837477a5a8b5

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22b9fb4f1c7d
part 1 - Rename *SlotCount to *SlotsSize in a few places. r=iain
https://hg.mozilla.org/integration/autoland/rev/cd45698fb194
part 2 - Don't include space for argument slots in safepoints. r=iain
https://hg.mozilla.org/integration/autoland/rev/1c4503c9996b
part 3 - Use unsigned integers for some offsets. r=iain
https://hg.mozilla.org/integration/autoland/rev/cc3833b385cd
part 4 - Tidy up frame offset and size calculations. r=iain
https://hg.mozilla.org/integration/autoland/rev/c012fb62e4fe
part 5 - Add frame pointer to IonJS/Warp frames. r=iain
https://hg.mozilla.org/integration/autoland/rev/709726da8eb0
part 6 - Rely on preserved frame pointer more after JIT ABI calls. r=iain

Comment 10

[Cosmin Sabou [:CosminS]]

Sorry about the backout, the failures were from Bug 1773265. Relanded your patches.

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/22b9fb4f1c7d
https://hg.mozilla.org/mozilla-central/rev/cd45698fb194
https://hg.mozilla.org/mozilla-central/rev/1c4503c9996b
https://hg.mozilla.org/mozilla-central/rev/cc3833b385cd
https://hg.mozilla.org/mozilla-central/rev/c012fb62e4fe
https://hg.mozilla.org/mozilla-central/rev/709726da8eb0

Comment 12

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Pulsebot from comment #9)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22b9fb4f1c7d
part 1 - Rename *SlotCount to *SlotsSize in a few places. r=iain== Change summary for alert #34553 (as of Fri, 17 Jun 2022 15:09:21 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
7%	assorted-dom	macosx1015-64-shippable-qr	fission webrender	18.40 -> 17.08

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34553

https://hg.mozilla.org/integration/autoland/rev/cd45698fb194
part 2 - Don't include space for argument slots in safepoints. r=iain
https://hg.mozilla.org/integration/autoland/rev/1c4503c9996b
part 3 - Use unsigned integers for some offsets. r=iain
https://hg.mozilla.org/integration/autoland/rev/cc3833b385cd
part 4 - Tidy up frame offset and size calculations. r=iain
https://hg.mozilla.org/integration/autoland/rev/c012fb62e4fe
part 5 - Add frame pointer to IonJS/Warp frames. r=iain
https://hg.mozilla.org/integration/autoland/rev/709726da8eb0
part 6 - Rely on preserved frame pointer more after JIT ABI calls. r=iain