Closed Bug 1341263 Opened 3 years ago Closed 3 years ago

Optimize null/undefined comparisons more in CompareIC

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.