bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Recover |this| argument ahead of bailouts.

RESOLVED FIXED in mozilla35

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

unspecified
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

To handle Scalar Replacement of new objects (Bug 1064358), we need to be able to recover the |this| slot ahead of bailouts, as it can be introspected by the stack iteration.  The idea would be similar to Bug 1062869 which added support for recovering argument slots.
Created attachment 8489596 [details] [diff] [review]
IonMonkey: Make |this| recoverable.
Attachment #8489596 - Flags: review?(shu)
Comment on attachment 8489596 [details] [diff] [review]
IonMonkey: Make |this| recoverable.

Review of attachment 8489596 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/CompileInfo.h
@@ +485,5 @@
>      // definition can be optimized away as long as we can compute its values.
>      bool isRecoverableOperand(uint32_t slot) const {
> +        // The |this| can be recovered.
> +        if (slot == thisSlot())
> +            return true;

I also guarded this by returning true if this is not a function.

Comment 3

4 years ago
Comment on attachment 8489596 [details] [diff] [review]
IonMonkey: Make |this| recoverable.

Review of attachment 8489596 [details] [diff] [review]:
-----------------------------------------------------------------

This part looks fine, but I'm confused: where's the actual code that does the recovery? What RInstruction is going to be encoded for the this slot?

::: js/src/jit/CompileInfo.h
@@ +485,5 @@
>      // definition can be optimized away as long as we can compute its values.
>      bool isRecoverableOperand(uint32_t slot) const {
> +        // The |this| can be recovered.
> +        if (slot == thisSlot())
> +            return true;

So you're saying this is (slot == thisSlot() && !fun_) ?

::: js/src/jsinfer.cpp
@@ +4062,5 @@
>      Vector<uint32_t, 32> pcOffsets(cx);
>      for (ScriptFrameIter iter(cx); !iter.done(); ++iter) {
>          pcOffsets.append(iter.script()->pcToOffset(iter.pc()));
> +
> +        // This frame has no this.

Nit: this would read better if you quoted 'this' like "This frame has no 'this' value"
Attachment #8489596 - Flags: review?(shu)
(In reply to Shu-yu Guo [:shu] from comment #3)
> This part looks fine, but I'm confused: where's the actual code that does
> the recovery?

The actual code is part of Bug 1062869, the MaybeReadFallback is used in maybeRead to fill the recover instruction vector.  This vector is then stored on the activation, and reused during the bailout.

> What RInstruction is going to be encoded for the this slot?

In theory, this could be anything.  The most common case would be MCreateThisWithTemplate, as done with MCreateThisWithTemplate as part of Bug 1064358.

> ::: js/src/jit/CompileInfo.h
> @@ +485,5 @@
> >      // definition can be optimized away as long as we can compute its values.
> >      bool isRecoverableOperand(uint32_t slot) const {
> > +        // The |this| can be recovered.
> > +        if (slot == thisSlot())
> > +            return true;
> 
> So you're saying this is (slot == thisSlot() && !fun_) ?

I am saying:

    bool isRecoverableOperand(uint32_t slot) const {
        if (!funMaybeLazy())
            return true;

        // The |this| can be recovered.
        if (slot == thisSlot())
            return true;

Comment 5

4 years ago
Comment on attachment 8489596 [details] [diff] [review]
IonMonkey: Make |this| recoverable.

Review of attachment 8489596 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/CompileInfo.h
@@ +485,5 @@
>      // definition can be optimized away as long as we can compute its values.
>      bool isRecoverableOperand(uint32_t slot) const {
> +        // The |this| can be recovered.
> +        if (slot == thisSlot())
> +            return true;

Please add a comment for the !funMaybeLazy() case.
Attachment #8489596 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/98267da0fc9e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.