Closed Bug 1378189 Opened 3 years ago Closed 10 months ago

Support derived constructors in Ion

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox75 --- fixed

People

(Reporter: tcampbell, Assigned: anba)

References

(Blocks 3 open bugs)

Details

Attachments

(12 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
We should support derived class constructors in Ion.

Opcodes to support:
  - JSOP_SUPERFUN
  - JSOP_SUPERCALL
  - JSOP_SPREADSUPERCALL
  - JSOP_CHECKTHIS
  - JSOP_CHECKTHISREINIT
  - JSOP_CHECKRETURN
  - JSOP_SPREADNEW (not required, but related)

The existing support for JSOP_SUPERCALL in Ion needs to be reviewed before being re-enabled. This operation behaves similar to JSOP_NEW but not all code checks for both correctly. Bailout code needs more test cases.

Another problem to address is TemplateObjects. Ion tries to use templateObjects to pass pre-allocated objects as the |this| hidden parameter of JSOP_NEW calls, but this isn't valid for JSOP_SUPERCALL with current semantics (See https://bugzilla.mozilla.org/show_bug.cgi?id=1169746#c15).
Blocks: 1167472, ares-6
Priority: -- → P3
Blocks: 1541708

Iain, you might be a good person to tackle this (in the future) since you've done a deep dive on templateObjects, constructors and calling conventions.

Assignee: nobody → andrebargull
Status: NEW → ASSIGNED

This is based on the existing support for JSOP_LAMBDA and JSOP_LAMBDA_ARROW.

Depends on D58776

Using only a vm-call for this operation should be okay for now.

Depends on D58777

This operation is similar to JSOP_GETINTRINSIC, where we can bake in a constant
value for known prototypes. In the future we may want to assign a type-set to
this operation, similar to JSOP_GETINTRINSIC, but for now we don't need more
exact type information here.

Depends on D58778

Changes the already present MCheckReturn class to match the baseline
implementation. MCheckReturn::foldsTo didn't fold away known types, so some
basic folding was directly added to IonBuilder::jsop_checkreturn().

Depends on D58779

Includes support for foldsTo, so we can optimise away the instruction for
known types.

Depends on D58780

Counterpart to JSOP_CHECKTHIS with a similar implementation.

Depends on D58781

Reads the prototype inline when possible, otherwise performs an OOL vm-call.

Depends on D58782

The note in IonBuilder::inspectOpcode() doesn't seem to apply anymore, probably
when we switched to CacheIR for call opcodes, the bailout issue noted there
doesn't matter anymore.

Inlining derived class constructors is enabled in the next part.

Depends on D58783

Passes the target JSFunction through to patchInlinedReturn, so we don't
emit MReturnFromCtor for derived class constructors. (Using MReturnFromCtor
for derived class constructors leads to repeated bailouts, because the computed
this-value is a magic type, which isn't a valid return type for MReturnFromCtor.)

Depends on D58784

Baseline code has explicit pre- and post-write barriers, but for Ion we should be
able to reuse the existing MPostWriteBarrier. Only pre-write barriers require
an explicit MacroAssembler call when emitting the assembly code.

Depends on D58786

  • JSOP_SPREADSUPERCALL and JSOP_SPREADNEW should be handled in a separate bug, because we can't easily reuse the existing CodeGenerator code for JSOP_SPREADCALL due to register shortage on x86. (It looks fixable, but may require to copy most of the existing code instead of being able to reuse it as is.)
  • Support for template objects to properly inline this-object creation is also not yet implemented, but should probably also happen in a separate bug.
Blocks: 1557765
Attachment #9118816 - Attachment description: Bug 1378189 - Part 1: Support JSOP_CHECKCLASSHERITAGE in Ion. r=jandem! → Bug 1378189 - Part 1: Support JSOp::CheckClassHeritage in Ion. r=jandem!
Attachment #9118817 - Attachment description: Bug 1378189 - Part 2: Support JSOP_FUNWITHPROTO in Ion. r=jandem! → Bug 1378189 - Part 2: Support JSOp::FunWithProto in Ion. r=jandem!
Attachment #9118818 - Attachment description: Bug 1378189 - Part 3: Support JSOP_OBJWITHPROTO in Ion. r=jandem! → Bug 1378189 - Part 3: Support JSOP::ObjWithProto in Ion. r=jandem!
Attachment #9118819 - Attachment description: Bug 1378189 - Part 4: Support JSOP_BUILTINPROTO in Ion. r=jandem! → Bug 1378189 - Part 4: Support JSOp::BuiltinProto in Ion. r=jandem!
Attachment #9118820 - Attachment description: Bug 1378189 - Part 5: Support JSOP_CHECKRETURN in Ion. r=jandem! → Bug 1378189 - Part 5: Support JSOp::CheckReturn in Ion. r=jandem!
Attachment #9118822 - Attachment description: Bug 1378189 - Part 7: Support JSOP_CHECKTHIS in Ion. r=jandem! → Bug 1378189 - Part 7: Support JSOp::CheckThis in Ion. r=jandem!
Attachment #9118823 - Attachment description: Bug 1378189 - Part 8: Support JSOP_CHECKTHISREINIT in Ion. r=jandem! → Bug 1378189 - Part 8: Support JSOp::CheckThisReinit in Ion. r=jandem!
Attachment #9118824 - Attachment description: Bug 1378189 - Part 9: Support JSOP_SUPERFUN in Ion. r=jandem! → Bug 1378189 - Part 9: Support JSOp::SuperFun in Ion. r=jandem!
Attachment #9118825 - Attachment description: Bug 1378189 - Part 10: Support JSOP_SUPERCALL in Ion. r=jandem! → Bug 1378189 - Part 10: Support JSOp::SuperCall in Ion. r=jandem!
Attachment #9118827 - Attachment description: Bug 1378189 - Part 12: Support JSOP_DERIVEDCONSTRUCTOR in Ion. r=jandem! → Bug 1378189 - Part 12: Support JSOp::DerivedConstructor in Ion. r=jandem!
Attachment #9118828 - Attachment description: Bug 1378189 - Part 13: Support JSOP_INITHOMEOBJECT in Ion. r=jandem! → Bug 1378189 - Part 13: Support JSOp::InitHomerObject in Ion. r=jandem!
Attachment #9118822 - Attachment description: Bug 1378189 - Part 7: Support JSOp::CheckThis in Ion. r=jandem! → Bug 1378189 - Part 6: Support JSOp::CheckThis in Ion. r=jandem!
Attachment #9118823 - Attachment description: Bug 1378189 - Part 8: Support JSOp::CheckThisReinit in Ion. r=jandem! → Bug 1378189 - Part 7: Support JSOp::CheckThisReinit in Ion. r=jandem!
Attachment #9118824 - Attachment description: Bug 1378189 - Part 9: Support JSOp::SuperFun in Ion. r=jandem! → Bug 1378189 - Part 8: Support JSOp::SuperFun in Ion. r=jandem!
Attachment #9118825 - Attachment description: Bug 1378189 - Part 10: Support JSOp::SuperCall in Ion. r=jandem! → Bug 1378189 - Part 9: Support JSOp::SuperCall in Ion. r=jandem!
Attachment #9118826 - Attachment description: Bug 1378189 - Part 11: Inline derived class constructors. r=jandem! → Bug 1378189 - Part 10: Inline derived class constructors. r=jandem!
Attachment #9118827 - Attachment description: Bug 1378189 - Part 12: Support JSOp::DerivedConstructor in Ion. r=jandem! → Bug 1378189 - Part 11: Support JSOp::DerivedConstructor in Ion. r=jandem!
Attachment #9118828 - Attachment description: Bug 1378189 - Part 13: Support JSOp::InitHomerObject in Ion. r=jandem! → Bug 1378189 - Part 12: Support JSOp::InitHomerObject in Ion. r=jandem!
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/56fe3e112600
Part 1: Support JSOp::CheckClassHeritage in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/e5babca4b855
Part 2: Support JSOp::FunWithProto in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/3c317670cb6e
Part 3: Support JSOP::ObjWithProto in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/be0da60bc0a3
Part 4: Support JSOp::BuiltinProto in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/72ea14062355
Part 5: Support JSOp::CheckReturn in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/ea42ded983e0
Part 6: Support JSOp::CheckThis in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/364196604df4
Part 7: Support JSOp::CheckThisReinit in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/0c208946524b
Part 8: Support JSOp::SuperFun in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/0a019885a638
Part 9: Support JSOp::SuperCall in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/de2b5e72dda0
Part 10: Inline derived class constructors. r=jandem
https://hg.mozilla.org/integration/autoland/rev/12169adecf49
Part 11: Support JSOp::DerivedConstructor in Ion. r=jandem
https://hg.mozilla.org/integration/autoland/rev/22c51dbffee2
Part 12: Support JSOp::InitHomerObject in Ion. r=jandem
You need to log in before you can comment on or make changes to this bug.