904827 - IonMonkey: Implement IC setter calls for JSNative setters

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Eric Faust [:efaust]]

Currently, we only generate stubs to call StrictPropertyOp setters, which isn't really all that useful for the DOM, whose objects are littered with JSNative setters with the new bindings.

Comment 1

[Eric Faust [:efaust]]

Attachment: Part 1: Change OOLNativeGetterExitFrame to OOLNativeExitFrame

Comment 2

[Eric Faust [:efaust]]

Attachment: Part 2: Implement cacheing of JSNative setters in SetPropertyIC

Comment 3

[Kannan Vijayan [:djvj]]

Comment on attachment 790498 [details] [diff] [review]
Part 1: Change OOLNativeGetterExitFrame to OOLNativeExitFrame

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

::: js/src/jit/IonFrames.cpp
@@ +937,5 @@
> +    if (frame.isOOLNative()) {
> +        IonOOLNativeExitFrameLayout *oolnative = frame.exitFrame()->oolNativeExit();
> +        gc::MarkIonCodeRoot(trc, oolnative->stubCode(), "ion-ool-native-code");
> +        size_t len = oolnative->argc() + 2;
> +        gc::MarkValueRootRange(trc, len, oolnative->vp(), "ion-ool-native-args");

For clarity's sake, I think this would be better split into a single MarkValueRoot(oolnative->vp()), plus an additional MarkValueRootRange(oolnative->argc() + 1, oolnative->thisp());

Alternatively, the IonFrames-ARCH.h files can be modified to explicitly insist that thisv and argvp immediately follow the callee result value stored in the frame.

Comment 4

[Kannan Vijayan [:djvj]]

Comment on attachment 790499 [details] [diff] [review]
Part 2: Implement cacheing of JSNative setters in SetPropertyIC

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

::: js/src/jit/IonCaches.cpp
@@ +1935,5 @@
> +    if (!shape || !IsCacheableProtoChain(obj, holder))
> +        return false;
> +
> +    if (!shape->writable())
> +        return false;

writable() is an attribute that is only relevant for data properties, not accessor (i.e. getter/setter) properties.

Drop this check.

@@ +1958,5 @@
> +    if (shape->hasDefaultSetter())
> +        return false;
> +
> +    if (!shape->writable())
> +        return false;

Remove writable() check.  See above.

Comment 5

[Eric Faust [:efaust]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/587ec77a2373
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f82c0e76f2d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/587ec77a2373
https://hg.mozilla.org/mozilla-central/rev/6f82c0e76f2d