Closed Bug 1613594 Opened 5 years ago Closed 5 years ago

Decouple IonBuilder from the Ion backend

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox74 --- wontfix
firefox75 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(9 files, 1 obsolete file)

(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details

IonBuilder is used for MIR building but also represents the off-thread compilation job. We should decouple this so IonBuilder is just the MIR builder (stack-allocated) so that we can more easily add a second builder.

There's a similar issue with MIRGenerator: it's IonBuilder's base class (and used for Wasm compilation) and is also used by the backend.

Priority: -- → P2

IonBuilder is a better fit for this data. MIRGenerator is used by the backend
and is also used for Wasm compilation and WarpBuilder in the future.

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED

This MIRGenerator flag is not used for inline builders so there's no need to set
it. With later patches in this stack there will be a single MIRGenerator per
compilation so this will become a bit more obvious.

Depends on D62007

For now MIRGenerator and IonBuilder still have the same lifetime, but later
parts will change IonBuilder to a stack-allocated class with a much shorter
lifetime than MIRGenerator.

Some MIRGenerator fields that are accessed frequently by IonBuilder are now
cached in IonBuilder to avoid an extra dereference.

Depends on D62008

We now no longer allocate a new MIRGenerator for each inline builder. This was
pretty confusing because only the outermost one survived and was used by the
backend.

MIRGenerator's CompileInfo field was renamed from info_ to outerInfo_ to make
this a bit clearer.

Depends on D62009

For now IonCompileTask just forwards to IonBuilder, but that will change in a
later patch.

Depends on D62010

This will let us shorten the IonBuilder lifetime later.

Depends on D62011

The pendingEdges_ field no longer has to use Maybe<> because we now always
call IonBuilder's destructor.

Depends on D62012

There's some follow-up work left: make stack-allocated IonBuilder scope even shorter and move IonCompileTask out of IonBuilder.h/cpp, but let's get this part done first.

This way we're also guaranteed to get a consistent value per compilation.

Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/146518d1ebf2 part 1 - Move abortedPreliminaryGroups Vector from MIRGenerator to IonBuilder. r=iain https://hg.mozilla.org/integration/autoland/rev/f396891236a0 part 2 - Only set the safeForMinorGC_ flag on the outermost builder. r=iain https://hg.mozilla.org/integration/autoland/rev/c5029fed3987 part 3 - Stop using MIRGenerator as base class for IonBuilder. r=iain https://hg.mozilla.org/integration/autoland/rev/854f4d27a3f1 part 4 - Use one MIRGenerator per compilation. r=iain https://hg.mozilla.org/integration/autoland/rev/7b925b0d13e8 part 5 - Add IonCompileTask class and use it instead of IonBuilder for off-thread tasks. r=iain https://hg.mozilla.org/integration/autoland/rev/50402c669262 part 6 - Change IonCompileTask to not require an IonBuilder. r=iain https://hg.mozilla.org/integration/autoland/rev/1bc78d155678 part 7 - Stop heap-allocating IonBuilder. r=iain https://hg.mozilla.org/integration/autoland/rev/fb651fbf6aaa part 8 - Rename ionBuilder memory reporter to ionCompileTask. r=iain https://hg.mozilla.org/integration/autoland/rev/64c6f3ce6465 part 9 - Speed up MIRGenerator::isOptimizationTrackingEnabled(). r=iain
Attached file Bug 1613594 follow-up fix for 32-bit x86 builds. (obsolete) (deleted) —
Pushed by opoprus@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0e498dfd4119 follow-up fix for 32-bit x86 builds. on a CLOSED TREE
Attachment #9125779 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: