Bug 82135 - DFG int-to-double conversion should be revealed to CSE
Summary: DFG int-to-double conversion should be revealed to CSE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-24 15:28 PDT by Filip Pizlo
Modified: 2012-03-25 16:51 PDT (History)
3 users (show)

See Also:


Attachments
work in progress (22.32 KB, patch)
2012-03-24 15:33 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
work in progress (29.92 KB, patch)
2012-03-24 15:59 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (33.97 KB, patch)
2012-03-24 20:02 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (33.35 KB, patch)
2012-03-25 00:52 PDT, Filip Pizlo
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
the patch (34.45 KB, patch)
2012-03-25 15:35 PDT, Filip Pizlo
oliver: review+
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
the patch (34.36 KB, patch)
2012-03-25 15:58 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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