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+

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