Bug 94408 - DFG CSE should be more honest about when it changed the IR
Summary: DFG CSE should be more honest about when it changed the IR
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 94205
  Show dependency tree / graph
 
Reported: 2012-08-17 19:15 PDT by Filip Pizlo
Modified: 2012-08-17 23:01 PDT (History)
0 users

See Also:

Attachments
the patch (2.14 KB, patch)
2012-08-17 19:26 PDT, Filip Pizlo 		ggaren: review+
Details | Formatted Diff | Diff
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-08-17 19:15:40 PDT 
Previously, the CSE would sometimes lie and say that it changed nothing.  But that can lead to humorous inconsistencies. For example:

a: GetGlobalVar(thingy)
b: CheckStructure(@a, 0x1234)
c: GetGlobalVar(thingy)
d: CheckStructure(@c, 0x2345)

This code will never complete, since there is no way that 'thingy' can simultaneously pass both structure checks.  But previously, we wouldn't realize this, because even though the CSE would modify the code to be:

a: GetGlobalVar(thingy)
b: CheckStructure(@a, 0x1234)
c: Phantom()
d: CheckStructure(@a, 0x2345)

It wouldn't let the optimization fixpoint know, and the optimization fixpoint wouldn't rerun the CFA.

Currently this "bug" is asymptomatic but it would become symptomatic if we started relying more the CFA results of things like CheckStructure.
Comment 1 Filip Pizlo 2012-08-17 19:26:17 PDT 
Created attachment 159257 [details]
the patch
Comment 2 Geoffrey Garen 2012-08-17 22:57:45 PDT 
Comment on attachment 159257 [details]
the patch

r=me
Comment 3 Filip Pizlo 2012-08-17 23:01:56 PDT 
Landed in http://trac.webkit.org/changeset/125964