Port Baseline TypeOf IC to CacheIR

RESOLVED FIXED in Firefox 55



2 years ago
2 years ago


(Reporter: jandem, Assigned: evilpie)


(Blocks 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)


(Whiteboard: [qf:p1])


(3 attachments, 1 obsolete attachment)

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).
Whiteboard: [qf]
Whiteboard: [qf] → [qf:p1]
Assignee: nobody → evilpies
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.
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?
Posted patch TypeOf object (obsolete) — Splinter Review

This also optimizes the Ion OOL path. I think it would even make sense to add a TypeOf object LIR variant.
Attachment #8862129 - Flags: review?(jdemooij)
Attachment #8863407 - Attachment is obsolete: true
Attachment #8863431 - Flags: review?(jdemooij)
Attachment #8862129 - Attachment description: Basic support → Primitive typeof CacheIR support
Attachment #8863433 - Flags: review?(jdemooij)
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
Attachment #8862129 - Flags: review?(jdemooij) → review+
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.
Attachment #8863431 - Flags: review?(jdemooij) → review+
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.
Attachment #8863433 - Flags: review?(jdemooij) → review+
Pushed by evilpies@gmail.com:
Primitive typeof CacheIR support. r=jandem
Ion/MacroAssembler typeof object improvements. r=jandem
Typeof object CacheIR support. r=jandem
You need to log in before you can comment on or make changes to this bug.