1350263 - Port Baseline TypeOf IC to CacheIR

Status: RESOLVED FIXED

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

Description

[Jan de Mooij [:jandem]]

Some problems with the current Baseline code:

* We don't optimize the "object" and "function" cases (even though these are very common).

* When we have an object emulating undefined, we attach a stub that checks for undefined but not for the object-emulating-undefined case so we keep attaching stubs (I noticed this in bug 1349298).

Comment 1

[Tom S [:evilpie]]

Attachment: Primitive typeof CacheIR support

Comment 2

[Tom S [:evilpie]]

So for objects, we can possibly handle everything but wrapped objects inline, because of wrapped objects that emulate-undefined. The SharedIC compare stub doesn't consider this flag for proxies though. Or we use callWithABI and TypeOfObjectOperation.

Comment 3

[Jan de Mooij [:jandem]]

I think it would be nice to handle everything other than wrapped objects inline. Maybe we could move that inline path into the MacroAssember and use it also for Ion's OOL path?

Comment 4

[Tom S [:evilpie]]

Attachment: TypeOf object

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eef5ffbd2f09222878d70b56c4890a8d3cdb2b06

This also optimizes the Ion OOL path. I think it would even make sense to add a TypeOf object LIR variant.

Comment 5

[Tom S [:evilpie]]

Attachment: Ion/MacroAssembler typeof changes

Comment 6

[Tom S [:evilpie]]

Attachment: Typeof object CacheIR support

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 8862129 [details] [diff] [review]
Primitive typeof CacheIR support

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

Cool, thanks for doing this.

::: js/public/Value.h
@@ +1336,5 @@
>      uint32_t toPrivateUint32() const { return value().toPrivateUint32(); }
>  
>      uint64_t asRawBits() const { return value().asRawBits(); }
>      JSValueType extractNonDoubleType() const { return value().extractNonDoubleType(); }
> +    JSValueType type() const { return value().type(); }

This method is not used I think

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8863431 [details] [diff] [review]
Ion/MacroAssembler typeof changes

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

::: js/src/jit/MacroAssembler.cpp
@@ +1456,5 @@
> +    branchPtr(Assembler::Zero, Address(scratch, offsetof(js::Class, cOps)),
> +                   ImmPtr(nullptr), isObject);
> +
> +    loadPtr(Address(scratch, offsetof(js::Class, cOps)), scratch);
> +    branchPtr(Assembler::Zero, Address(scratch, offsetof(js::ClassOps, call)),

Nit: Assembler::Equal

::: js/src/jit/SharedIC.cpp
@@ +1780,5 @@
>          masm.moveValue(BooleanValue(op == JSOP_STRICTNE), R0);
>          EmitReturnFromIC(masm);
>      } else {
>          // obj != undefined only where !obj->getClass()->emulatesUndefined()
> +        // XXX We should use branchIfObjectEmulatesUndefined here, but not sure which scratch to use.

Maybe for now just push the object register? Or we could use R1 and restore it later... We should port this code.

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8863433 [details] [diff] [review]
Typeof object CacheIR support

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

Looks good. This will help a lot of framework code.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/12921b015564
Primitive typeof CacheIR support. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4bd22f374df
Ion/MacroAssembler typeof object improvements. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/68042c9ca8dc
Typeof object CacheIR support. r=jandem

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/12921b015564
https://hg.mozilla.org/mozilla-central/rev/f4bd22f374df
https://hg.mozilla.org/mozilla-central/rev/68042c9ca8dc