Bug 82135

Summary: DFG int-to-double conversion should be revealed to CSE
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gns, pnormand, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
work in progress
none
work in progress
none
the patch
none
the patch
webkit-ews: commit-queue-
the patch
oliver: review+, gyuyoung.kim: commit-queue-
the patch none

Description Filip Pizlo 2012-03-24 15:28:03 PDT
Currently if we have an integer variable, like:

var x = 1;

And we use it in multiple places in a context that requires conversion to double, like:

... = x * 0.5;
... = sqrt(x);

Then we will convert it to double on each of those uses.  That's wasteful, and we should be able to remember that the conversion had already been performed by separately tracking the converted-to-double version of the value.

We have an even more gross problem if we do the following:

var x = o.f; // o.f predicted int
... = x * 0.5;
... = array[x];

The first statement will format x as a JSValue.  The second statement will speculate that x is a number and convert it to a double in-place. The third statement will then end up doing horrible slow things to try to perform an array access using a double as an index.

Again, if we had a way of separately tracking the converted-to-double form of this integer variable, this would not be a problem.
Comment 1 Filip Pizlo 2012-03-24 15:33:49 PDT
Created attachment 133653 [details]
work in progress
Comment 2 Filip Pizlo 2012-03-24 15:59:07 PDT
Created attachment 133657 [details]
work in progress

I'm almost ready to start testing it...
Comment 3 Filip Pizlo 2012-03-24 20:02:16 PDT
Created attachment 133668 [details]
the patch
Comment 4 Filip Pizlo 2012-03-25 00:52:09 PDT
Created attachment 133673 [details]
the patch

Rebased patch.
Comment 5 Early Warning System Bot 2012-03-25 00:58:23 PDT
Comment on attachment 133673 [details]
the patch

Attachment 133673 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12129342
Comment 6 Early Warning System Bot 2012-03-25 01:00:13 PDT
Comment on attachment 133673 [details]
the patch

Attachment 133673 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12134270
Comment 7 Filip Pizlo 2012-03-25 01:02:37 PDT
(In reply to comment #6)
> (From update of attachment 133673 [details])
> Attachment 133673 [details] did not pass qt-ews (qt):
> Output: http://queues.webkit.org/results/12134270

Ooops, I need to write the 32-bit version of compileInt32ToDouble. :-(
Comment 8 Filip Pizlo 2012-03-25 15:35:37 PDT
Created attachment 133695 [details]
the patch

Fix 32-bit.
Comment 9 Gyuyoung Kim 2012-03-25 15:51:40 PDT
Comment on attachment 133695 [details]
the patch

Attachment 133695 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12134479
Comment 10 Philippe Normand 2012-03-25 15:52:25 PDT
Comment on attachment 133695 [details]
the patch

Attachment 133695 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12129524
Comment 11 Filip Pizlo 2012-03-25 15:53:29 PDT
(In reply to comment #9)
> (From update of attachment 133695 [details])
> Attachment 133695 [details] did not pass efl-ews (efl):
> Output: http://queues.webkit.org/results/12134479

Fix on the way ... it appears that in the process of moving #if's around, I broke 64-bit.
Comment 12 Build Bot 2012-03-25 15:57:59 PDT
Comment on attachment 133695 [details]
the patch

Attachment 133695 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12131501
Comment 13 Filip Pizlo 2012-03-25 15:58:14 PDT
Created attachment 133696 [details]
the patch

Putting up for EWS.
Comment 14 Filip Pizlo 2012-03-25 16:51:22 PDT
Landed in http://trac.webkit.org/changeset/112040