1357759 - Implement unboxed, typed, proxy and megamorphic stubs for JSOP_IN

Status: RESOLVED FIXED

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

Description

[Tom S [:evilpie]]

In bug 1357468 I added support for unboxed objects to the HasOwn. Adding this to |in| would be similar. This shows up on Speedometer and even on sunspider.

Additional with the landing in bug 1344469 I also added a megamorphic stub that now handles unboxed objects.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1357759 - Cleanup HasPropIRGenerator unboxed cases

This cleans up the HasPropIRGenerator to support unboxed, expando
unboxed, and typed object cases, as well cleaning up the megamorphic
handling.

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

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1357759 - Support proxies ICs for JSOP_IN

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

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1357759 - Support megamorphic IC for JSOP_IN

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

Comment 5

[Ted Campbell [:tcampbell]]

This can probably use more automated testcases.

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 8912474 [details]
Bug 1357759 - Cleanup HasPropIRGenerator unboxed cases

https://reviewboard.mozilla.org/r/183794/#review189854

Beautiful.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8912475 [details]
Bug 1357759 - Support proxies ICs for JSOP_IN

https://reviewboard.mozilla.org/r/183796/#review189858

::: js/src/jit/CacheIR.cpp:2600
(Diff revision 1)
>          return false;
>      }
>      RootedObject obj(cx_, &val_.toObject());
>      ObjOperandId objId = writer.guardIsObject(valId);
>  
> -    // Optimize DOM Proxies for JSOP_HASOWN
> +    // Optimize DOM Proxies

Pre-existing nit: remove "DOM" here? The code works for all proxies AFAICS.

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8912476 [details]
Bug 1357759 - Support megamorphic IC for JSOP_IN

https://reviewboard.mozilla.org/r/183798/#review189862

Nice. It would be good to check we have tests for megamorphic JSOP_IN - evilpie added some for JSOP_HASOWN in jit-test/tests/cacheir/hasown.js

::: js/src/jit/CacheIRCompiler.cpp:2573
(Diff revision 1)
>      masm.setupUnalignedABICall(scratch);
>      masm.loadJSContext(scratch);
>      masm.passABIArg(scratch);
>      masm.passABIArg(obj);
>      masm.passABIArg(idVal.scratchReg());
> -    masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, HasOwnNativeDataProperty));
> +    if (hasOwn) {

Nit: no {}

Comment 10

[Ted Campbell [:tcampbell]]

Comment on attachment 8912475 [details]
Bug 1357759 - Support proxies ICs for JSOP_IN

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

Comment 11

[Ted Campbell [:tcampbell]]

Comment on attachment 8912476 [details]
Bug 1357759 - Support megamorphic IC for JSOP_IN

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d394ed31deb4
Cleanup HasPropIRGenerator unboxed cases r=jandem
https://hg.mozilla.org/integration/autoland/rev/fdab9ee63195
Support proxies ICs for JSOP_IN r=jandem
https://hg.mozilla.org/integration/autoland/rev/9db13c9ff60e
Support megamorphic IC for JSOP_IN r=jandem

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d394ed31deb4
https://hg.mozilla.org/mozilla-central/rev/fdab9ee63195
https://hg.mozilla.org/mozilla-central/rev/9db13c9ff60e