Optimize null/undefined comparisons more in CompareIC

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

(Blocks 1 bug)

unspecified
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

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: nobody → evilpies
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!)
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a00935f57ef
Optimize null/undefined comparisons more in CompareIC. r=jandem
https://hg.mozilla.org/mozilla-central/rev/0a00935f57ef
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.