Bug 19484

Summary:

More instructions needs to use temporary registers

Product:

WebKit

Reporter:

Cameron Zwarich (cpst) <zwarich>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WORKSFORME

Severity:

Normal

CC:

barraclough, ggaren, mjs, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

All

Attachments:

Description	Flags
Patch towards a fix	none
Fix for AddNode and SubNode	none
Patch to improve codegen	none
Patch for binary operations	none

Cameron Zwarich (cpst)

Reported 2008-06-11 09:32:13 PDT

The code var a = 1; print(a + ++a); prints 4 instead of 3, because we are not moving 'a' to a temporary registers. I fixed this for assignment nodes, but a similar thing needs to be done for a lot more nodes. I'll try to fix this for binary operations today and hope that it doesn't cause too much of a performance regression.

Attachments
Patch towards a fix (8.87 KB, patch) 2008-06-13 21:32 PDT, Cameron Zwarich (cpst)	no flags	Details Formatted Diff Diff
Fix for AddNode and SubNode (8.74 KB, patch) 2008-06-14 00:16 PDT, Cameron Zwarich (cpst)	no flags	Details Formatted Diff Diff
Patch to improve codegen (11.99 KB, patch) 2008-06-14 22:16 PDT, Cameron Zwarich (cpst)	no flags	Details Formatted Diff Diff
Patch for binary operations (67.01 KB, patch) 2008-06-15 01:06 PDT, Cameron Zwarich (cpst)	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Cameron Zwarich (cpst)

Comment 1 2008-06-13 21:32:36 PDT

Created attachment 21692 [details] Patch towards a fix Here is the first step towards a solution. It makes codegen of expressions that might need a temporary for the LHS smarter when the RHS is a constant. It's not that big of a deal now, but it will matter more when we do this for more opcodes.

Maciej Stachowiak

Comment 2 2008-06-13 21:34:45 PDT

Comment on attachment 21692 [details] Patch towards a fix r=me

Cameron Zwarich (cpst)

Comment 3 2008-06-13 21:42:10 PDT

Landed in r34528.

Cameron Zwarich (cpst)

Comment 4 2008-06-14 00:16:39 PDT

Created attachment 21695 [details] Fix for AddNode and SubNode Here is a fix for the AddNode and SubNode case. It only adds one instruction in SunSpider, for the line dnaInput = dnaInput + dnaInput + dnaInput; in global code in regexp-dna.

Oliver Hunt

Comment 5 2008-06-14 00:38:50 PDT

Comment on attachment 21695 [details] Fix for AddNode and SubNode r=me

Cameron Zwarich (cpst)

Comment 6 2008-06-14 00:54:28 PDT

Landed in r34535. I plan to do the multiplication nodes next, and finally the bit operations.

Oliver Hunt

Comment 7 2008-06-14 01:05:37 PDT

Alas, you'll also need to deal unto the relational operators.

Cameron Zwarich (cpst)

Comment 8 2008-06-14 22:16:03 PDT

Created attachment 21705 [details] Patch to improve codegen This patch makes codegen not use a temporary for the LHS if the RHS is a local variable. This improves code generation on SunSpider in a few places. It was a small regression on my machine, but Maciej says it is fine on his.

Maciej Stachowiak

Comment 9 2008-06-14 22:18:55 PDT

Comment on attachment 21705 [details] Patch to improve codegen r=me

Cameron Zwarich (cpst)

Comment 10 2008-06-15 01:06:13 PDT

Created attachment 21706 [details] Patch for binary operations Here is a fix for binary operations. I will add tests soon, similar to the other cases I have already landed, but I wanted to put this up for review first.

Maciej Stachowiak

Comment 11 2008-06-15 01:30:14 PDT

Comment on attachment 21706 [details] Patch for binary operations r=me

Cameron Zwarich (cpst)

Comment 12 2008-06-15 02:33:32 PDT

Landed in r34562. There are still cases with FunctionCallResolveNode and ForInNode to fix, but I don't know of any others.

Cameron Zwarich (cpst)

Comment 13 2008-06-24 14:38:50 PDT

Comment on attachment 21692 [details] Patch towards a fix Clearing review flag.

Cameron Zwarich (cpst)

Comment 14 2008-06-24 14:39:00 PDT

Comment on attachment 21695 [details] Fix for AddNode and SubNode Clearing review flag.

Cameron Zwarich (cpst)

Comment 15 2008-06-24 14:39:12 PDT

Comment on attachment 21705 [details] Patch to improve codegen Clearing review flag.

Cameron Zwarich (cpst)

Comment 16 2008-06-24 14:39:24 PDT

Comment on attachment 21706 [details] Patch for binary operations Clearing review flag.

Alexey Proskuryakov

Comment 17 2010-06-11 17:43:48 PDT

Is this bug still actionable?

Gavin Barraclough

Comment 18 2012-09-11 18:45:10 PDT

All examples given now work, looks like all bugs being tracked here are fixed. Works for me, if I've missed anything please reopen.

Note You need to log in before you can comment on or make changes to this bug.