316879 - Faster code for pure int operations

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

This is a separated bug not to forget about the patch from comment 121414#c63 which is not a part of the last patch for bug 121414.

For details, see comments 55 - 63 in the bug.

Comment 1

[Igor Bukanov]

Attachment: Updated patch to reflect CVS changes

I hope this would not interrupt the work on bug 121414.

Comment 2

[Brendan Eich [:brendan]]

Thanks, I wasn't going to forget about those, but I was hoping to work them into a future patch.  We can take your patch now and do it the other way, for sure, but I would like to land the patch for bug 121414 first.  That makes you do the merge, which isn't really fair.  But I thought I'd ask ;-).  I can do the merge and land the other way 'round.  Thoughts?

/be

Comment 3

[Igor Bukanov]

(In reply to comment #2)
> but I would like to land the patch for bug 121414 first.

That is OK: that "review?" had ping as hidden agenda in any case ;)

Comment 4

[Igor Bukanov]

Dependency on bug 121414 is to know when to update the patch.

Comment 5

[Igor Bukanov]

Attachment: Update patch and -0 fix

Here is a patch update after landing of attachment 203610 [details] [diff] [review] from bug 121414 plus fix with proper handling of 0 negation when the previous patch would leave 0 coming from int jsval as 0.

The patch shows the following improvements for test case from attachment 145601 [details] for optimized build of jsshell on Pentium II 360MHz laptop with Fedora Core 4/ GCC 4.0.1:

+-------------+-------------+-------------+---------+
|    Test     |   Before    |    After    | Speedup |
|-------------|-------------|-------------|---------|
|   Test 1    |   2986 ms   |   1814 ms   |   65%   |
|-------------|-------------|-------------|---------|
|   Test 2    |   2633 ms   |   1666 ms   |   58%   |
|-------------|-------------|-------------|---------|
|   Test 3    |   2912 ms   |   2359 ms   |   23%   |
+-------------+-------------+-------------+---------+

These are much higher speedups then ones that were before landing of threaded interpreted patch. In fact the combined effect of these patches is almost 2 times faster execution of test1. Perhaps I do not need to upgrade the laptop after all;)

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 203709 [details] [diff] [review]
Update patch and -0 fix

Nice!

/be

Comment 7

[Igor Bukanov]

I committed the fix but I keep the bug as assigned until 121414 would be marked as fixed.

Comment 8

[Igor Bukanov]

I just reverted my commit: the patch contained bogus assertion in JSOP_NEG implementation: even if initially rval is not int jsval, the result still may fit to int jsval in cases like when rval is "1".

Proper patch in a moment.

Comment 9

[Igor Bukanov]

Attachment: Patch with proper optimization of JSOP_NEG

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 203735 [details] [diff] [review]
Patch with proper optimization of JSOP_NEG

r=me

/be

Comment 11

[Igor Bukanov]

The last patch is in CVS

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Fresh compile with MSVC2003 using the testcases from bug 121414.
+-------------+-------------+-------------+---------+
|    Test     |   Before    |    After    | Speedup |
|-------------|-------------|-------------|---------|
|   Test 1    |   312 ms    |   250 ms    |  19.8%  |
|-------------|-------------|-------------|---------|
|   Test 2    |   281 ms    |   234 ms    |  16.7%  |
|-------------|-------------|-------------|---------|
|   Test 3    |   360 ms    |   344 ms    |   4.4%  |
+-------------+-------------+-------------+---------+

Nice boost in tests 1 and 2 again :)

Comment 13

[Igor Bukanov]

Marking as fixed: I forgot to do that around 2005-11-21.