Bug 82008 - DFG double voting may be overzealous in the case of variables that end up being used as integers
Summary: DFG double voting may be overzealous in the case of variables that end up bei...
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-22 20:21 PDT by Filip Pizlo
Modified: 2012-03-24 17:46 PDT (History)
1 user (show)

See Also:

Attachments
work in progress (32.94 KB, patch)
2012-03-22 20:37 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (33.53 KB, patch)
2012-03-22 22:25 PDT, Filip Pizlo 		msaboff: review+
Details | Formatted Diff | Diff
the patch (36.71 KB, patch)
2012-03-23 16:15 PDT, Filip Pizlo 		barraclough: review+
Details | Formatted Diff | Diff
the patch (35.92 KB, patch)
2012-03-23 19:12 PDT, Filip Pizlo 		barraclough: review+
Details | Formatted Diff | Diff
the patch (33.97 KB, patch)
2012-03-23 20:16 PDT, Filip Pizlo 		oliver: review+
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-03-22 20:21:31 PDT 
Patch forthcoming.
Comment 1 Filip Pizlo 2012-03-22 20:37:10 PDT 
Created attachment 133423 [details]
work in progress

Still working on it but at least it compiles now.
Comment 2 Filip Pizlo 2012-03-22 22:25:40 PDT 
Created attachment 133431 [details]
the patch
Comment 3 Michael Saboff 2012-03-23 14:08:28 PDT 
Comment on attachment 133431 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133431&action=review

r+ with the one fix.

> Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:199
> +            mergeDefaultFlags(node);

Need to or this into changed.
Comment 4 Filip Pizlo 2012-03-23 16:15:06 PDT 
Created attachment 133581 [details]
the patch
Comment 5 Filip Pizlo 2012-03-23 19:12:48 PDT 
Created attachment 133611 [details]
the patch
Comment 6 Filip Pizlo 2012-03-23 20:16:13 PDT 
Created attachment 133615 [details]
the patch

Removed the EscapesAsInt portion of the patch because it was causing perf regressions, and the case where it was beneficial was sort of contrived to begin with.
Comment 7 Geoffrey Garen 2012-03-24 15:21:35 PDT 
> and the case where it was beneficial was sort of contrived to begin with.

... Interesting. FWIW, kind of struck me that way, too.
Comment 8 Filip Pizlo 2012-03-24 15:29:03 PDT 
(In reply to comment #7)
> > and the case where it was beneficial was sort of contrived to begin with.
> 
> ... Interesting. FWIW, kind of struck me that way, too.

Yeah Gavin had to convince me to trust the numbers on this one. ;-)
Comment 9 Filip Pizlo 2012-03-24 17:46:45 PDT 
Landed in http://trac.webkit.org/changeset/112015