1567438 - Clean up Ion bailouts

Status: RESOLVED FIXED

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

Description

[Jan de Mooij [:jandem]]

We can now resume in the interpreter and this simplifies a number of things, for example we no longer have to worry about unsynced R0/R1 stack values.

However there's more to clean up, for example the enter-monitor-chain code can be removed if we monitor this Value in C++.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 1 - Replace the enter-monitor-chain bailout code with type monitoring in C++. r?tcampbell!,iain!

When we resumeAfter a JOF_TYPESET op, we had code to enter the type monitor
chain for the bailing op, passing it the top stack value. It's simpler to do
this monitoring in C++ in FinishFromBailout and then treat it as a regular
resumeAfter bailout.

The code for this will be simplified more later in the stack.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 2 - Clean up environment chain and prologue bailout handling. r?tcampbell!

We now always set the frame's environment chain to a non-null value and have
an explicit isPrologueBailout flag instead of relying on envChain == nullptr.

Depends on D38860

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 3 - Don't skip JSOP_LOOPENTRY ops for prologue bailouts. r?tcampbell!

The old code happened to work but didn't really make sense and I ran into some assertion
failures with other changes. Later patches will clean this up more.

Depends on D38861

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 4 - Don't set the frame's return value for noScriptRval scripts. r?tcampbell!

The old code happened to work for Baseline JIT bailouts because the JIT ignores
the return value slot for noScriptRval scripts, but when bailing out to the
interpreter we would have an optimized-out MagicValue in the return value slot
and get confused.

Depends on D38862

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 5 - Change IsIonInlinablePC to IsIonInlinableOp. r?tcampbell!

We need this for the next patch because we have a JSOp instead of a pc when
generating the interpreter. (In general it's nicer for functions to take a JSOp
instead of a PC because it can be used in strictly more contexts.)

Depends on D38863

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 6 - Save IC return offsets in interpreter code for IsIonInlinableOp ops. r?tcampbell!

We need this for interpreter bailouts for bailouts involving inlining.

Depends on D38864

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 7 - Some BaselineFrame changes. r?tcampbell!

This adds/changes some methods we need later on.

Depends on D38865

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 8 - Change Ion bailouts to resume in the interpreter instead of JIT. r?tcampbell!

There will be more clean up in later patches.

Depends on D38866

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 9 - Modernize BaselineBailoutInfo. r?tcampbell!

Use proper C++ constructors/destructors, use UniquePtr instead of js_free.

Depends on D38867

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 10 - Remove now-unnecessary AutoSuppressGC in FinishBailoutToBaseline. r?tcampbell!

Depends on D38868

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 11 - Add GetResumePC, make pc and op variables in InitFromBailout const. r?tcampbell!

Making variables in InitFromBailout immutable makes the code a lot easier to reason about.

Depends on D38869

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Combined patch

Comment 13

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 12 - Simplify type monitor code a bit and fix a now-stale comment. r?tcampbell!,iain!

Depends on D38870

Comment 14

[Jan de Mooij [:jandem]]

Attachment: Combined patch v2

Comment 15

[Jan de Mooij [:jandem]]

Attachment: Bug 1567438 part 13 - Modernize and simplify BaselineStackBuilder. r?tcampbell!

Instead of storing both buffer_ and header_ we now store just header_ (as UniquePtr).

This also removes HeaderSize() and uses sizeof(BaselineBailoutInfo) directly.

Depends on D38975

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/58ea17fc315e
part 1 - Replace the enter-monitor-chain bailout code with type monitoring in C++. r=tcampbell,iain
https://hg.mozilla.org/integration/autoland/rev/ff457522f563
part 2 - Clean up environment chain and prologue bailout handling. r=tcampbell,iain
https://hg.mozilla.org/integration/autoland/rev/ea48a9066430
part 3 - Don't skip JSOP_LOOPENTRY ops for prologue bailouts. r=tcampbell,iain
https://hg.mozilla.org/integration/autoland/rev/92cda183a416
part 4 - Don't set the frame's return value for noScriptRval scripts. r=tcampbell,iain
https://hg.mozilla.org/integration/autoland/rev/9267308e0b89
part 5 - Change IsIonInlinablePC to IsIonInlinableOp. r=tcampbell,iain
https://hg.mozilla.org/integration/autoland/rev/ffe515491b2a
part 6 - Save IC return offsets in interpreter code for IsIonInlinableOp ops. r=tcampbell,iain

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92c20b931f96
part 7 - Some BaselineFrame changes. r=tcampbell CLOSED TREE
https://hg.mozilla.org/integration/autoland/rev/3c3b12e8c597
part 8 - Change Ion bailouts to resume in the interpreter instead of JIT. r=tcampbell CLOSED TREE
https://hg.mozilla.org/integration/autoland/rev/9a1eeba0d050
part 9 - Modernize BaselineBailoutInfo. r=tcampbell CLOSED TREE
https://hg.mozilla.org/integration/autoland/rev/cc928df24bc5
part 10 - Remove now-unnecessary AutoSuppressGC in FinishBailoutToBaseline. r=tcampbell CLOSED TREE
https://hg.mozilla.org/integration/autoland/rev/6e65f403e1ce
part 11 - Add GetResumePC, make pc and op variables in InitFromBailout const. r=tcampbell CLOSED TREE
https://hg.mozilla.org/integration/autoland/rev/098b10e61991
part 12 - Simplify type monitor code a bit and fix a now-stale comment. r=tcampbell,iain CLOSED TREE
https://hg.mozilla.org/integration/autoland/rev/e7d2e7780cbd
part 13 - Modernize and simplify BaselineStackBuilder. r=tcampbell CLOSED TREE

Comment 18

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/58ea17fc315e
https://hg.mozilla.org/mozilla-central/rev/ff457522f563
https://hg.mozilla.org/mozilla-central/rev/ea48a9066430
https://hg.mozilla.org/mozilla-central/rev/92cda183a416
https://hg.mozilla.org/mozilla-central/rev/9267308e0b89
https://hg.mozilla.org/mozilla-central/rev/ffe515491b2a
https://hg.mozilla.org/mozilla-central/rev/92c20b931f96
https://hg.mozilla.org/mozilla-central/rev/3c3b12e8c597
https://hg.mozilla.org/mozilla-central/rev/9a1eeba0d050
https://hg.mozilla.org/mozilla-central/rev/cc928df24bc5
https://hg.mozilla.org/mozilla-central/rev/6e65f403e1ce
https://hg.mozilla.org/mozilla-central/rev/098b10e61991
https://hg.mozilla.org/mozilla-central/rev/e7d2e7780cbd