Bug 20796

Summary: Combine op_eq and op_lesseq with op_jfalse
Product: WebKit Reporter: Gabor Loki <loki>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Enhancement CC: emacemac7, ggaren, mjs, zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
op_jneq and op_jnlesseq instructions
darin: review-
SunSpider result for op_jneq and op_jnlesseq instructions
none
op_jneq and op_jnlesseq instructions (v2)
none
SunSpider, V8, WindScorpion results (non-jit)
none
SunSpider, V8, WindScorpion results (JIT)
none
op_jneq and op_jnlesseq instructions (v3)
none
SunSpider, V8, WindScorpion results (non-jit)
none
SunSpider, V8, WindScorpion results (JIT) none

Description Gabor Loki 2008-09-12 07:49:59 PDT
I have found that op_eq and op_jfalse instructions appear 441683 times (0.72%) and op_lesseq and op_jfalse are 340196 times (0.55%) in SunSpider.

These two instruction pair mostly come up in condition fields, where the results of the binary operator never used after the jump.

So it is possible to introduce two super-instructions for those cases (like op_jnless).
I have implemented these op_jneq and op_jnlesseq instructions, and the result is 0.8% progression in SunSpider.
Comment 1 Gabor Loki 2008-09-12 07:51:14 PDT
Created attachment 23369 [details]
op_jneq and op_jnlesseq instructions
Comment 2 Gabor Loki 2008-09-12 07:51:57 PDT
Created attachment 23370 [details]
SunSpider result for op_jneq and op_jnlesseq instructions
Comment 3 Maciej Stachowiak 2008-09-13 14:19:53 PDT
Looks like some good results, though we will also need CTI / native code implementations of these.

Comment 4 Darin Adler 2008-09-21 11:38:36 PDT
Maciej, this seems to be related to the work you're doing right now. Maybe you could review and land this?
Comment 5 Darin Adler 2008-09-21 13:55:05 PDT
Comment on attachment 23369 [details]
op_jneq and op_jnlesseq instructions

Looks great.

Now that CTI is landed, we need the CTI versions -- we can't add an opcode without that. So review- because of that issue.
Comment 6 Gabor Loki 2008-11-19 05:39:18 PST
Created attachment 25263 [details]
op_jneq and op_jnlesseq instructions (v2)

I have added the CTI part of these instructions.
The SunSpider results are the follwing on Qt-linux:
 - NON-JIT: 1.007% progression
 - JIT: 1.001% propression

I am going to attach the full SunSpider, V8, WindScorpion results.
Comment 7 Gabor Loki 2008-11-19 05:43:48 PST
Err. Wrong number. Those numbers are from sunspider-compare-results, but they are not percentages.

The valid numbers:
 - NON-JIT: 0.7% progression
 - JIT: 0.1% propression

Comment 8 Gabor Loki 2008-11-19 08:27:37 PST
Created attachment 25269 [details]
SunSpider, V8, WindScorpion results (non-jit)

Here are the non-JIT results of the last patch on Qt-linux.
Comment 9 Gabor Loki 2008-11-19 08:28:14 PST
Created attachment 25270 [details]
SunSpider, V8, WindScorpion results (JIT)

JIT results of the last patch on Qt-linux.
Comment 10 Gabor Loki 2008-12-17 07:02:36 PST
Created attachment 26094 [details]
op_jneq and op_jnlesseq instructions (v3)

I have updated the previous patch on ToT.
The results are almost the same. I will attach them later.
Comment 11 Gabor Loki 2008-12-17 07:58:21 PST
Created attachment 26095 [details]
SunSpider, V8, WindScorpion results (non-jit)
Comment 12 Gabor Loki 2008-12-17 07:59:15 PST
Created attachment 26096 [details]
SunSpider, V8, WindScorpion results (JIT)
Comment 13 Eric Seidel (no email) 2009-05-11 05:24:22 PDT
Is this bug even still relevant?  It's been sitting in the queue for 5 months...
Comment 14 Gabor Loki 2009-05-12 08:23:32 PDT
This is still valid. There is about 1-2% progression on x86 without JIT. I am going to send the patch and the results tomorrow.
Comment 15 Gabor Loki 2009-05-13 04:15:21 PDT
It looks like op_jnlesseq instruction have been added in r43401. The op_jneq does not give more speedup on x86 without JIT. So, this bug should be closed.
Comment 16 Eric Seidel (no email) 2009-05-19 19:50:42 PDT
Closing per loki's comment.
Comment 17 Eric Seidel (no email) 2009-05-19 19:51:00 PDT
Comment on attachment 26094 [details]
op_jneq and op_jnlesseq instructions (v3)

obsoleting patch.