Bug 82008

Summary: DFG double voting may be overzealous in the case of variables that end up being used as integers
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
work in progress
none
the patch
msaboff: review+
the patch
barraclough: review+
the patch
barraclough: review+
the patch oliver: review+

Filip Pizlo
Reported 2012-03-22 20:21:31 PDT
Patch forthcoming.
Attachments
work in progress (32.94 KB, patch)
2012-03-22 20:37 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (33.53 KB, patch)
2012-03-22 22:25 PDT, Filip Pizlo 		msaboff: review+
DetailsFormatted DiffDiff
the patch (36.71 KB, patch)
2012-03-23 16:15 PDT, Filip Pizlo 		barraclough: review+
DetailsFormatted DiffDiff
the patch (35.92 KB, patch)
2012-03-23 19:12 PDT, Filip Pizlo 		barraclough: review+
DetailsFormatted DiffDiff
the patch (33.97 KB, patch)
2012-03-23 20:16 PDT, Filip Pizlo 		oliver: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2012-03-22 20:37:10 PDT
Created attachment 133423 [details] work in progress Still working on it but at least it compiles now.
Filip Pizlo
Comment 2 2012-03-22 22:25:40 PDT
Created attachment 133431 [details] the patch
Michael Saboff
Comment 3 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.
Filip Pizlo
Comment 4 2012-03-23 16:15:06 PDT
Created attachment 133581 [details] the patch
Filip Pizlo
Comment 5 2012-03-23 19:12:48 PDT
Created attachment 133611 [details] the patch
Filip Pizlo
Comment 6 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.
Geoffrey Garen
Comment 7 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.
Filip Pizlo
Comment 8 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. ;-)
Filip Pizlo
Comment 9 2012-03-24 17:46:45 PDT
Landed in http://trac.webkit.org/changeset/112015
Note You need to log in before you can comment on or make changes to this bug.