If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Optimize null/undefined comparisons more in CompareIC

RESOLVED FIXED in Firefox 54

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

(Blocks: 1 bug)

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
Created attachment 8839441 [details] [diff] [review]
Optimize null/undefined comparisons more in CompareIC

This is not nice code, but improves ARES-6 in the shell quite a bit. (Probably less after ion compiling lexical scopes though, but at least DoCompareFallback doesn't show in profiles anymore).

Handling null/undefined == something is actually pretty easy, there is only null/undefined and objects emulating undefined to consider. I hope we can clean this code up after porting it to CacheIR.

We don't use hasStub anymore, because the stub is specialized for one side (left or right) and one type (null or undefined), this is all pretty horrible, but I can't currently work on fixing this properly.
Attachment #8839441 - Flags: review?(hv1989)
(Assignee)

Updated

7 months ago
Assignee: nobody → evilpies
(Assignee)

Updated

7 months ago
Attachment #8839441 - Flags: review?(hv1989) → review?(jdemooij)
Comment on attachment 8839441 [details] [diff] [review]
Optimize null/undefined comparisons more in CompareIC

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

LGTM. Agreed we should port this to CacheIR soon so we can see which other cases show up on the web.

::: js/src/jit/SharedIC.cpp
@@ +1556,5 @@
>              stub->addNewStub(objectStub);
>              return true;
>          }
>  
> +        if ((lhs.isNull() || lhs.isUndefined()) || (rhs.isNull() || rhs.isUndefined())) {

Pre-existing nit: maybe lhs.isNullOrUndefined() || rhs.isNullOrUndefined() ?
Attachment #8839441 - Flags: review?(jdemooij) → review+
(And sorry for the delay!)

Comment 3

7 months ago
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a00935f57ef
Optimize null/undefined comparisons more in CompareIC. r=jandem

Comment 4

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0a00935f57ef
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.