Closed Bug 1648005 Opened 2 years ago Closed 2 years ago

Warp: handle the unreachable-OSR-loop case

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

It's possible for the OSR-pc to be only reachable via a try-catch block. IonBuilder has code to handle that but WarpBuilder doesn't. I'm running into this with some local Warp patches.

We now crash instead of silently using an offset of 0 (meaning start of the
IonScript's JIT code) for the OSR entry point if the MOsrEntry is missing
for some reason.

Some changes preparing for the next patch.

Depends on D80874

If we see a forward branch in a catch/finally block to a not-yet-initialized
location outside it, don't perform Ion/Warp OSR at LoopHeads following it in
the script.

The abort in IonBuilder can then be turned into an assertion. This helps
WarpBuilder because we don't want any non-OOM aborts during MIR building.

Depends on D80875

Attachment #9158890 - Attachment description: Bug 1648005 part 2 - Clean up some BytecodeAnalysis code. r?iain! → Bug 1648005 part 2 - Change loopHeadInCatchOrFinally to loopHeadCanOsr. r?iain!
Attachment #9158891 - Attachment description: Bug 1648005 part 3 - Change BytecodeAnalysis to avoid the unreachable-OSR-loop case. r?iain! → Bug 1648005 part 3 - Change BytecodeAnalysis to prevent the unreachable-OSR-loop case. r?iain!

The analysis marked code after a try-catch as always-reachable to simplify MIR
building. Now that we compute reachability of LoopHeads, there's not a lot we gain
from that complexity anymore.

We can then also remove the MGotoWithFake code and the JSOp::Try bytecode operand.

Depends on D80876

The first try-catch in the test now terminates control flow after the previous patch
and we then ilooped because inIon() never returned true in the loops after that
(these loops are only reachable through the catch-block).

The patch splits the test in a few different functions to fix this.

Depends on D81029

Severity: normal → N/A
Priority: -- → P1
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a4078dbbe41
part 1 - Use Maybe<> for osrEntryOffset_ and add a release-assert. r=iain
https://hg.mozilla.org/integration/autoland/rev/a8753560dc09
part 2 - Change loopHeadInCatchOrFinally to loopHeadCanOsr. r=iain
https://hg.mozilla.org/integration/autoland/rev/c3121ce23d2c
part 3 - Change BytecodeAnalysis to prevent the unreachable-OSR-loop case. r=iain
https://hg.mozilla.org/integration/autoland/rev/dfce0fc62953
part 4 - Remove some try-catch code we no longer need. r=iain
https://hg.mozilla.org/integration/autoland/rev/5bd462d99f91
part 5 - Fix a test. r=iain
Blocks: 1649900
You need to log in before you can comment on or make changes to this bug.