701966 - IonMonkey: Compile JSOP_NAME

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

Necessary for benchmarks.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: WIP (bf524b56351f)

Barely working patch to add a CompileInfo slot to keep track of the current scope chain, compile NAME and CALLNAME.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: patch (edde637d2661)

Comment 3

[Brian Hackett [Laid off!]]

Attachment: patch (4b6b2d6421f9)

Rebased, with tweak to avoid loading the scope chain in frames which will not use it (would be redundant with dead code elimination).

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 585417 [details] [diff] [review]
patch (4b6b2d6421f9)

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

r=me with some small refactorings to MIR/LIR

::: js/src/ion/CodeGenerator.cpp
@@ +562,2 @@
>      Label mismatched;
> +    for (uint32 i = 1; i < CountArgSlots(gen->info().fun()); i++) {

It'd be nice if this 1 here and in (i - 1) below were a constant somewhere, akin to THIS_SLOT. like FIRST_ARGUMENT or something

::: js/src/ion/IonBuilder.cpp
@@ +382,5 @@
> +
> +    // The scope chain is only tracked in scripts that have NAME opcodes which
> +    // will try to access the scope. For other scripts, the scope instructions
> +    // will be dead but without a dead code elimination pass their code will
> +    // still be generated.

We currently have such a pass, but the slot will be held live by resume pointers. That's okay though, it won't generate any LIR and snapshots have special encoding for constants.

::: js/src/ion/LIR-Common.h
@@ +776,5 @@
>      }
>  };
>  
> +// Materialize a JSObject stored in an interpreter frame for OSR.
> +class LOsrObject : public LInstructionHelper<1, 1, 0>

Similar comment here, whether this can be LOsrScopeChain or something.

@@ +1161,5 @@
> +        return getDef(0);
> +    }
> +};
> +
> +class LCallGetPropertyOrName : public LVMCallInstructionHelper<LDefinition::BOX, BOX_PIECES, 1, 0>

This should be split up as the MIR.

::: js/src/ion/MIR.h
@@ +668,5 @@
> +    }
> +
> +  public:
> +    INSTRUCTION_HEADER(Callee);
> +    static MCallee *New() { return new MCallee(); }

nit: local style is
    static MCallee *New() {
        return new MCallee();
    }

@@ +1812,5 @@
>  };
>  
> +// MIR representation of a JSObject pointer on the OSR StackFrame.
> +// The pointer is indexed off of OsrFrameReg.
> +class MOsrObject : public MAryInstruction<1>

Do we plan on this being used for things other than the scope chain? If not, can we call this something like MOsrScopeChain (and then get rid of the frameOffset field)?

@@ +2392,5 @@
> +    enum AccessKind {
> +        PROPERTY,
> +        NAMETYPEOF,
> +        NAME
> +    };

Here we should have two separate opcodes, MGetPropertyGeneric (or just MGetProperty) and MGetName or something. You could share everything in a base class, like MPropertyNameInstruction or whatever, but we should have at least two distinct ops.

Comment 5

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/projects/ionmonkey/rev/b26b0782dd94

Comment 6

[Paul Wright]

Can this be closed?