Rewrite and simplify control flow logic in Ion, remove IonControlFlow
Categories
(Core :: JavaScript Engine: JIT, task, P1)
Tracking
()
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.
Assignee | ||
Comment 1•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Comment 3•5 years ago
|
||
Depends on D52633
Assignee | ||
Comment 4•5 years ago
|
||
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
Assignee | ||
Comment 5•5 years ago
|
||
Depends on D52635
Assignee | ||
Comment 6•5 years ago
|
||
Depends on D52636
Comment 8•5 years ago
|
||
Backed out 5 changesets (Bug 1595476) for causing memory leaks CLOSED TREE
Push with failureS: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=277184391&resultStatus=testfailed%2Cbusted%2Cexception&revision=f38ea7496f7f46a0ad17c8e8403a2b2a71f372e7
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=277184390&repo=autoland&lineNumber=1607
Backout: https://hg.mozilla.org/integration/autoland/rev/5dee96491b4bb83d7042b715f52239d58891a7e3
Assignee | ||
Comment 9•5 years ago
|
||
Bah, IonBuilder not having its destructor called bites again. It's for the GSNCache.
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3922ccdc1fe9
https://hg.mozilla.org/mozilla-central/rev/3c4bccda2d54
https://hg.mozilla.org/mozilla-central/rev/03783d54b398
https://hg.mozilla.org/mozilla-central/rev/823ab2a11fa8
https://hg.mozilla.org/mozilla-central/rev/f9313eb69e9d
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Description
•