1377051 - Support JSOP_SETPROP_SUPER in Baseline

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Ted Campbell [:tcampbell]]

There are only a few remaining opcodes for Baseline support of ES6 Classes.
JSOP_SETPROP_SUPER
JSOP_STRICTSETPROP_SUPER
JSOP_SETELEM_SUPER
JSOP_STRICTSETELEM_SUPER

The complexities out-weight the usefulness, so use a VMCall to implement. Will also include a jit-test for some of the special complexities that SetPropIRGenerator does not support.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1377051 - Support JSOP_SETPROP_SUPER in Baseline

Review commit: https://reviewboard.mozilla.org/r/153570/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153570/

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1377051 - Support JSOP_SETELEM_SUPER in Baseline

Review commit: https://reviewboard.mozilla.org/r/153572/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153572/

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8882444 [details]
Bug 1377051 - Support JSOP_SETPROP_SUPER in Baseline

https://reviewboard.mozilla.org/r/153570/#review158936

Thanks for adding the tests.

::: js/src/vm/Interpreter.cpp:2874
(Diff revision 1)
>  
> -
>      ReservedRooted<Value> receiver(&rootValue0, REGS.sp[-3]);
>      ReservedRooted<JSObject*> obj(&rootObject0, &REGS.sp[-2].toObject());
>      ReservedRooted<Value> rval(&rootValue1, REGS.sp[-1]);
> -    ReservedRooted<jsid> id(&rootId0, NameToId(script->getName(REGS.pc)));
> +    ReservedRooted<PropertyName*> id(&rootName0, script->getName(REGS.pc));

Nit: rename id -> name

::: js/src/vm/Interpreter.cpp:5263
(Diff revision 1)
> +js::SetPropertySuper(JSContext* cx, HandleObject obj, HandleValue receiver,
> +                     HandlePropertyName name, HandleValue rval, bool strict)
> +{
> +    RootedId id(cx, NameToId(name));
> +    ObjectOpResult result;
> +    if (!SetProperty(cx, obj, name, rval, receiver, result))

Nit: can pass id instead of name, I think the name version will do NameToId(name) a second time.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8882445 [details]
Bug 1377051 - Support JSOP_SETELEM_SUPER in Baseline

https://reviewboard.mozilla.org/r/153572/#review158942

Do we have decent tests for this that run in Baseline?

Comment 5

[Ted Campbell [:tcampbell]]

(In reply to Jan de Mooij [:jandem] from comment #4)
> Comment on attachment 8882445 [details]
> Bug 1377051 - Support JSOP_SETELEM_SUPER in Baseline
> 
> https://reviewboard.mozilla.org/r/153572/#review158942
> 
> Do we have decent tests for this that run in Baseline?
Tests are included in previous patch tests.

Comment 6

[Ted Campbell [:tcampbell]]

Comment on attachment 8882444 [details]
Bug 1377051 - Support JSOP_SETPROP_SUPER in Baseline

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153570/diff/1-2/

Comment 7

[Ted Campbell [:tcampbell]]

Comment on attachment 8882445 [details]
Bug 1377051 - Support JSOP_SETELEM_SUPER in Baseline

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153572/diff/1-2/

Comment 8

[Ted Campbell [:tcampbell]]

Comment on attachment 8882444 [details]
Bug 1377051 - Support JSOP_SETPROP_SUPER in Baseline

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153570/diff/2-3/

Comment 9

[Ted Campbell [:tcampbell]]

Comment on attachment 8882445 [details]
Bug 1377051 - Support JSOP_SETELEM_SUPER in Baseline

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153572/diff/2-3/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52c00746ebc4
Support JSOP_SETPROP_SUPER in Baseline r=jandem
https://hg.mozilla.org/integration/autoland/rev/2319ae1c0f8a
Support JSOP_SETELEM_SUPER in Baseline r=jandem

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/52c00746ebc4
https://hg.mozilla.org/mozilla-central/rev/2319ae1c0f8a