Closed Bug 1595476 Opened 5 years ago Closed 5 years ago

Rewrite and simplify control flow logic in Ion, remove IonControlFlow

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 1 obsolete file)

Ion compilation has always relied on reconstructing the script's control flow graph from the bytecode. This requires a lot of complexity for things like loops (including resolving continues, breaks), switch-statements (the structure/overlapping of case/default labels), etc. It also makes it harder to change the bytecode structure and requires source notes for many opcodes.

Since bug 1310155 control flow handling is a separate pass, but the complexity and BCE/Ion coupling is still there.

And it's largely unnecessary. There's a simpler strategy:

  • Compile the bytecode from first to last instruction.
  • When we have a forward branch (JSOP_GOTO, JSOP_IFEQ, JSOP_CASE, etc) we end the block but put it on a "pending blocks" list.
  • When we get to a jump target op, we check for matching pending blocks and link successors if needed.
  • Loops still require some special logic, but this is much simpler than what we do now.

My prototype passes all shell tests, has similar performance on SunSpider/Kraken/Octane and has a super sweet diffstat:

13 files changed, 845 insertions(+), 3426 deletions(-)

There's probably more code (and source notes..) we can remove as follow-up.

Attached file Bug 1595476 - WIP (obsolete) (deleted) —
Priority: -- → P1
Depends on: 1595690
Depends on: 1595691
Blocks: 1595699

Instead of trying to understand the precise Control Flow Graph, we now construct
MIR more like how a baseline compiler does it: whenever we have a forward jump
in the bytecode we add the block to a pendingBlocks list (keyed on the target
pc) and when we get to a jump target op we "link" any pending blocks for that
pc.

This patch also changes 'continues' in while/for-in/for-of loops to be more
similar to continues in for-loops and do-while loops. They're now just forward
jumps to the end of the loop body, instead of backward jumps to the branch at
the top that jumps to the condition. It's simpler and because they're now plain
forward branches the PendingBlock system handles them automatically.

We still always emit a jump target op for continues, even if there are no
continues. It's pretty easy to optimize this but that will be done in a
follow-up (bug 1595699) to not complicate this patch more. We can likely also
remove some source notes.

Depends on D52634

Depends on D52636

Blocks: 1597943
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0c9fc36b5c6e part 1 - Add clearAndCompact method to InlineMap/InlineSet. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/97633eed11db part 2 - Add hasTryFinally flag to IonBytecodeInfo. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/22b1766f2fd1 part 3 - Rewrite and simplify control flow logic in Ion for bytecode -> MIR compilation. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/e13e5cc4e2e7 part 4 - Remove now-almost-empty IonControlFlow.{h,cpp} files. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/f38ea7496f7f part 5 - Remove now-dead CFG memory reporter. r=tcampbell

Bah, IonBuilder not having its destructor called bites again. It's for the GSNCache.

Queued up for landing again.

Flags: needinfo?(jdemooij)
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3922ccdc1fe9 part 1 - Add clearAndCompact method to InlineMap/InlineSet. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/3c4bccda2d54 part 2 - Add hasTryFinally flag to IonBytecodeInfo. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/03783d54b398 part 3 - Rewrite and simplify control flow logic in Ion for bytecode -> MIR compilation. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/823ab2a11fa8 part 4 - Remove now-almost-empty IonControlFlow.{h,cpp} files. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/f9313eb69e9d part 5 - Remove now-dead CFG memory reporter. r=tcampbell
Blocks: 1598244
Blocks: 1598548
Blocks: 1598631
Blocks: 1599117
Attachment #9107835 - Attachment is obsolete: true
No longer regressions: 1602190
Blocks: WarpBuilder
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: