Closed Bug 1583172 Opened 6 months ago Closed 6 months ago

Define/set BaselineFrame::frameSize_ only in DEBUG builds

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(7 files)

When calling into the VM, BaselineFrame::frameSize_ is very similar to the frame size stored in the frame descriptor. The main difference is that frameSize_ does not include the size of VM function arguments when calling a VM wrapper so it's used for determining the number of Values on the BaselineFrame's local/expression stack. It's also more easily available given just a BaselineFrame* pointer.

The duplication has been bothering me for a while because it involves a store and (in some cases) some calculation for each VM call from Baseline code.

It's possible to make BaselineFrame::frameSize_ DEBUG-only and rename it (nice for assertions for now), make JSJitFrameIter compute the equivalent frame size, and pass the frame size explicitly to some VM calls where we don't want frame iteration. I have some prototype patches for this and it works nicely.

Depends on: 1583176
Depends on: 1583216

This just forwards to BaselineFrame for now but that will change.

Depends on D46911

The computed value is equivalent to BaselineFrame::frameSize_, but later in the stack
the BaselineFrame::frameSize_ field will be debug-only.

Depends on D46912

For now we just assert it matches the size stored in the frame.

Depends on D46913

One of the more tricky parts: Ion's NewBaselineFrameInspector needs to know the
frmae size for numValueSlots so this patch passes down a BaselineFrameWithSize
struct that stores both the frame and the size.

Depends on D46916

Blocks: 1583487
Priority: -- → P1
Attachment #9094851 - Attachment description: Bug 1583172 part 6 - Add a BaselineFrameWithSize struct and pass it down to Ion compilation. r?tcampbell! → Bug 1583172 part 6 - Pass frame size to Ion's NewBaselineFrameInspector. r?tcampbell!
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/92dca2ee7dd3
part 1 - Remove always-nullptr arguments from jit::Recompile. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/e94031916ebf
part 2 - Add baselineFrameNumValueSlots method to JSJitFrameIter and use it in a few places. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/71e47d3ce0f3
part 3 - Make JSJitFrameIter compute the Baseline frame size. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d337e9229327
part 4 - Pass frame size explicitly to jit::NormalSuspend. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/de54dcbfcf74
part 5 - Store frame size in BaselineBailoutInfo. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/690f3798bbaf
part 6 - Pass frame size to Ion's NewBaselineFrameInspector. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/4f2cba0b03be
part 7 - Rename BaselineFrame's frameSize_ field to debugFrameSize_ and define/set it only in debug builds. r=tcampbell
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0195e903299
part 1 - Remove always-nullptr arguments from jit::Recompile. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8a111dcc2a98
part 2 - Add baselineFrameNumValueSlots method to JSJitFrameIter and use it in a few places. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/c75007e8327a
part 3 - Make JSJitFrameIter compute the Baseline frame size. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/6b5de6b58a89
part 4 - Pass frame size explicitly to jit::NormalSuspend. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/50e944d9d99c
part 5 - Store frame size in BaselineBailoutInfo. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/5937ca265a0b
part 6 - Pass frame size to Ion's NewBaselineFrameInspector. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/b2feeb41df2b
part 7 - Rename BaselineFrame's frameSize_ field to debugFrameSize_ and define/set it only in debug builds. r=tcampbell
Flags: needinfo?(jdemooij)
You need to log in before you can comment on or make changes to this bug.