Title: [104016] trunk
Revision
104016
Author
[email protected]
Date
2012-01-03 23:26:42 -0800 (Tue, 03 Jan 2012)

Log Message

DFG: The assertion that a double-voted variable cannot become double-unvoted is wrong
https://bugs.webkit.org/show_bug.cgi?id=75516
<rdar://problem/10640266>

Source/_javascript_Core: 

Reviewed by Gavin Barraclough.
        
Removed the offending assertion, since it was wrong.  Also hardened the code to make
this case less likely by first having the propagator fixpoint converge, and then doing
double voting combined with a second fixpoint.  This is neutral on benchmarks and
fixes the assertion in a fairly low-risk way (i.e. we won't vote a variable double
until we've converged to the conclusion that it really is double).

* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::propagatePredictions):
* dfg/DFGVariableAccessData.h:
(JSC::DFG::VariableAccessData::tallyVotesForShouldUseDoubleFormat):

LayoutTests: 

Reviewed by Andy Estes.
        
Created a fuzzer that produces sufficiently awkward data flow that includes variables
that become either double, or integer, or mix of double and integer only after multiple
iterations of a fixpoint. This crashes the compiler prior to this patch, but works with
this patch.

* fast/js/dfg-double-vote-fuzz-expected.txt: Added.
* fast/js/dfg-double-vote-fuzz.html: Added.
* fast/js/script-tests/dfg-double-vote-fuzz.js: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (104015 => 104016)


--- trunk/LayoutTests/ChangeLog	2012-01-04 07:05:36 UTC (rev 104015)
+++ trunk/LayoutTests/ChangeLog	2012-01-04 07:26:42 UTC (rev 104016)
@@ -1,3 +1,20 @@
+2012-01-03  Filip Pizlo  <[email protected]>
+
+        DFG: The assertion that a double-voted variable cannot become double-unvoted is wrong
+        https://bugs.webkit.org/show_bug.cgi?id=75516
+        <rdar://problem/10640266>
+
+        Reviewed by Andy Estes.
+        
+        Created a fuzzer that produces sufficiently awkward data flow that includes variables
+        that become either double, or integer, or mix of double and integer only after multiple
+        iterations of a fixpoint. This crashes the compiler prior to this patch, but works with
+        this patch.
+
+        * fast/js/dfg-double-vote-fuzz-expected.txt: Added.
+        * fast/js/dfg-double-vote-fuzz.html: Added.
+        * fast/js/script-tests/dfg-double-vote-fuzz.js: Added.
+
 2012-01-03  Adam Barth  <[email protected]>
 
         Fix assert when foster parenting self-closing elements

Added: trunk/LayoutTests/fast/js/dfg-double-vote-fuzz-expected.txt (0 => 104016)


--- trunk/LayoutTests/fast/js/dfg-double-vote-fuzz-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-double-vote-fuzz-expected.txt	2012-01-04 07:26:42 UTC (rev 104016)
@@ -0,0 +1,266 @@
+This tests that the static analysis we use to infer if a variable should be a double doesn't crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Result value is 54939
+Result value is 55039
+Result value is 55039
+Result value is 55139
+Result value is 55039
+Result value is 55139
+Result value is 55139
+Result value is 55239
+Result value is 55039
+Result value is 55139
+Result value is 55139
+Result value is 55239
+Result value is 55139
+Result value is 55239
+Result value is 55239
+Result value is 55339
+Result value is 55821
+Result value is 55921
+Result value is 55921
+Result value is 56021
+Result value is 55921
+Result value is 56021
+Result value is 56021
+Result value is 56121
+Result value is 55921
+Result value is 56021
+Result value is 56021
+Result value is 56121
+Result value is 56021
+Result value is 56121
+Result value is 56121
+Result value is 56221
+Result value is 55828.5
+Result value is 55928.5
+Result value is 55928.5
+Result value is 56028.5
+Result value is 55928.5
+Result value is 56028.5
+Result value is 56028.5
+Result value is 56128.5
+Result value is 55928.5
+Result value is 56028.5
+Result value is 56028.5
+Result value is 56128.5
+Result value is 56028.5
+Result value is 56128.5
+Result value is 56128.5
+Result value is 56228.5
+Result value is 56710.5
+Result value is 56810.5
+Result value is 56810.5
+Result value is 56910.5
+Result value is 56810.5
+Result value is 56910.5
+Result value is 56910.5
+Result value is 57010.5
+Result value is 56810.5
+Result value is 56910.5
+Result value is 56910.5
+Result value is 57010.5
+Result value is 56910.5
+Result value is 57010.5
+Result value is 57010.5
+Result value is 57110.5
+Result value is 55834
+Result value is 55934
+Result value is 55934
+Result value is 56034
+Result value is 55934
+Result value is 56034
+Result value is 56034
+Result value is 56134
+Result value is 55934
+Result value is 56034
+Result value is 56034
+Result value is 56134
+Result value is 56034
+Result value is 56134
+Result value is 56134
+Result value is 56234
+Result value is 56716
+Result value is 56816
+Result value is 56816
+Result value is 56916
+Result value is 56816
+Result value is 56916
+Result value is 56916
+Result value is 57016
+Result value is 56816
+Result value is 56916
+Result value is 56916
+Result value is 57016
+Result value is 56916
+Result value is 57016
+Result value is 57016
+Result value is 57116
+Result value is 56723.5
+Result value is 56823.5
+Result value is 56823.5
+Result value is 56923.5
+Result value is 56823.5
+Result value is 56923.5
+Result value is 56923.5
+Result value is 57023.5
+Result value is 56823.5
+Result value is 56923.5
+Result value is 56923.5
+Result value is 57023.5
+Result value is 56923.5
+Result value is 57023.5
+Result value is 57023.5
+Result value is 57123.5
+Result value is 57605.5
+Result value is 57705.5
+Result value is 57705.5
+Result value is 57805.5
+Result value is 57705.5
+Result value is 57805.5
+Result value is 57805.5
+Result value is 57905.5
+Result value is 57705.5
+Result value is 57805.5
+Result value is 57805.5
+Result value is 57905.5
+Result value is 57805.5
+Result value is 57905.5
+Result value is 57905.5
+Result value is 58005.5
+Result value is 55887.5
+Result value is 55987.5
+Result value is 55987.5
+Result value is 56087.5
+Result value is 55987.5
+Result value is 56087.5
+Result value is 56087.5
+Result value is 56187.5
+Result value is 55987.5
+Result value is 56087.5
+Result value is 56087.5
+Result value is 56187.5
+Result value is 56087.5
+Result value is 56187.5
+Result value is 56187.5
+Result value is 56287.5
+Result value is 56769.5
+Result value is 56869.5
+Result value is 56869.5
+Result value is 56969.5
+Result value is 56869.5
+Result value is 56969.5
+Result value is 56969.5
+Result value is 57069.5
+Result value is 56869.5
+Result value is 56969.5
+Result value is 56969.5
+Result value is 57069.5
+Result value is 56969.5
+Result value is 57069.5
+Result value is 57069.5
+Result value is 57169.5
+Result value is 56777
+Result value is 56877
+Result value is 56877
+Result value is 56977
+Result value is 56877
+Result value is 56977
+Result value is 56977
+Result value is 57077
+Result value is 56877
+Result value is 56977
+Result value is 56977
+Result value is 57077
+Result value is 56977
+Result value is 57077
+Result value is 57077
+Result value is 57177
+Result value is 57659
+Result value is 57759
+Result value is 57759
+Result value is 57859
+Result value is 57759
+Result value is 57859
+Result value is 57859
+Result value is 57959
+Result value is 57759
+Result value is 57859
+Result value is 57859
+Result value is 57959
+Result value is 57859
+Result value is 57959
+Result value is 57959
+Result value is 58059
+Result value is 56782.5
+Result value is 56882.5
+Result value is 56882.5
+Result value is 56982.5
+Result value is 56882.5
+Result value is 56982.5
+Result value is 56982.5
+Result value is 57082.5
+Result value is 56882.5
+Result value is 56982.5
+Result value is 56982.5
+Result value is 57082.5
+Result value is 56982.5
+Result value is 57082.5
+Result value is 57082.5
+Result value is 57182.5
+Result value is 57664.5
+Result value is 57764.5
+Result value is 57764.5
+Result value is 57864.5
+Result value is 57764.5
+Result value is 57864.5
+Result value is 57864.5
+Result value is 57964.5
+Result value is 57764.5
+Result value is 57864.5
+Result value is 57864.5
+Result value is 57964.5
+Result value is 57864.5
+Result value is 57964.5
+Result value is 57964.5
+Result value is 58064.5
+Result value is 57672
+Result value is 57772
+Result value is 57772
+Result value is 57872
+Result value is 57772
+Result value is 57872
+Result value is 57872
+Result value is 57972
+Result value is 57772
+Result value is 57872
+Result value is 57872
+Result value is 57972
+Result value is 57872
+Result value is 57972
+Result value is 57972
+Result value is 58072
+Result value is 58554
+Result value is 58654
+Result value is 58654
+Result value is 58754
+Result value is 58654
+Result value is 58754
+Result value is 58754
+Result value is 58854
+Result value is 58654
+Result value is 58754
+Result value is 58754
+Result value is 58854
+Result value is 58754
+Result value is 58854
+Result value is 58854
+Result value is 58954
+PASS grandResult is 14578304
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/js/dfg-double-vote-fuzz.html (0 => 104016)


--- trunk/LayoutTests/fast/js/dfg-double-vote-fuzz.html	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-double-vote-fuzz.html	2012-01-04 07:26:42 UTC (rev 104016)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/fast/js/script-tests/dfg-double-vote-fuzz.js (0 => 104016)


--- trunk/LayoutTests/fast/js/script-tests/dfg-double-vote-fuzz.js	                        (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-double-vote-fuzz.js	2012-01-04 07:26:42 UTC (rev 104016)
@@ -0,0 +1,68 @@
+description(
+"This tests that the static analysis we use to infer if a variable should be a double doesn't crash."
+);
+
+var grandResult = 0;
+
+for (var i = 0; i < 256; ++i) {
+    code  = "function foo(o, a, b) {\n";
+    code += "    var x = 0;\n";
+    code += "    var c = a + b;\n";
+    for (var j = 0; j < 8; ++j)
+        code += "    var " + String.fromCharCode("d".charCodeAt(0) + j) + " = " + ((i & (1 << ((j / 2) | 0))) ? "0.5" : "0") + ";\n";
+    code += "    for (var __i = 0; __i < 10; ++__i) {\n";
+    code += "        if (a + b + 1 > __i) {\n";
+    code += "            c = d;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    code += "        if (a + b + 2 > __i) {\n";
+    code += "            c = e;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    code += "        if (a + b + 3 > __i) {\n";
+    code += "            c = f;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    code += "        if (a + b + 4 > __i) {\n";
+    code += "            c = g;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    code += "        if (a + b + 5 > __i) {\n";
+    code += "            c = h;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    code += "        if (a + b + 6 > __i) {\n";
+    code += "            c = i;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    code += "        if (a + b + 7 > __i) {\n";
+    code += "            c = j;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    code += "        if (a + b + 8 > __i) {\n";
+    code += "            c = k;\n";
+    code += "            x += c;\n";
+    code += "        }\n";
+    for (var j = 0; j < 8; ++j) {
+        code += "        if (a + b + " + (9 + j) + " > __i)\n";
+        code += "            " + String.fromCharCode("d".charCodeAt(0) + j) + " = __i + " + j + " + " + ((i & (1 << (((j / 2) | 0) + 4))) ? "0.5" : "0") + ";\n";
+    }
+    code += "    }\n";
+    code += "    return x + c;\n";
+    code += "}\n";
+    code += "\n";
+    code += "var result = 0;\n"
+    code += "for (var __j = 0; __j < 100; ++__j) {\n";
+    code += "    result += foo({}, __j, __j + 1);\n";
+    code += "}\n";
+    code += "\n";
+    code += "result";
+    
+    var theResult = eval(code);
+    debug("Result value is " + theResult);
+    grandResult += theResult;
+}
+
+shouldBe("grandResult", "14578304");
+
+

Modified: trunk/Source/_javascript_Core/ChangeLog (104015 => 104016)


--- trunk/Source/_javascript_Core/ChangeLog	2012-01-04 07:05:36 UTC (rev 104015)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-01-04 07:26:42 UTC (rev 104016)
@@ -1,5 +1,24 @@
 2012-01-03  Filip Pizlo  <[email protected]>
 
+        DFG: The assertion that a double-voted variable cannot become double-unvoted is wrong
+        https://bugs.webkit.org/show_bug.cgi?id=75516
+        <rdar://problem/10640266>
+
+        Reviewed by Gavin Barraclough.
+        
+        Removed the offending assertion, since it was wrong.  Also hardened the code to make
+        this case less likely by first having the propagator fixpoint converge, and then doing
+        double voting combined with a second fixpoint.  This is neutral on benchmarks and
+        fixes the assertion in a fairly low-risk way (i.e. we won't vote a variable double
+        until we've converged to the conclusion that it really is double).
+
+        * dfg/DFGPropagator.cpp:
+        (JSC::DFG::Propagator::propagatePredictions):
+        * dfg/DFGVariableAccessData.h:
+        (JSC::DFG::VariableAccessData::tallyVotesForShouldUseDoubleFormat):
+
+2012-01-03  Filip Pizlo  <[email protected]>
+
         REGRESSION (r98196-98236): Incorrect layout of iGoogle with RSS feeds
         https://bugs.webkit.org/show_bug.cgi?id=75303
         <rdar://problem/10633533>

Modified: trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp (104015 => 104016)


--- trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp	2012-01-04 07:05:36 UTC (rev 104015)
+++ trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp	2012-01-04 07:26:42 UTC (rev 104016)
@@ -768,13 +768,14 @@
 #if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
         m_count = 0;
 #endif
+        // Two stage process: first propagate predictions, then propagate while doing double voting.
+        
         do {
             m_changed = false;
             
             // Forward propagation is near-optimal for both topologically-sorted and
             // DFS-sorted code.
             propagatePredictionsForward();
-            doRoundOfDoubleVoting();
             if (!m_changed)
                 break;
             
@@ -784,7 +785,18 @@
             // found a sound solution and (2) short-circuits backward flow.
             m_changed = false;
             propagatePredictionsBackward();
+        } while (m_changed);
+        
+        do {
+            m_changed = false;
             doRoundOfDoubleVoting();
+            propagatePredictionsForward();
+            if (!m_changed)
+                break;
+            
+            m_changed = false;
+            doRoundOfDoubleVoting();
+            propagatePredictionsBackward();
         } while (m_changed);
     }
     

Modified: trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h (104015 => 104016)


--- trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h	2012-01-04 07:05:36 UTC (rev 104015)
+++ trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h	2012-01-04 07:26:42 UTC (rev 104016)
@@ -112,8 +112,8 @@
         
         bool newValueOfShouldUseDoubleFormat = shouldUseDoubleFormatAccordingToVote();
         if (!newValueOfShouldUseDoubleFormat) {
-            // Assert that we're monotonically converting to force-double.
-            ASSERT(!m_shouldUseDoubleFormat);
+            // We monotonically convert to double. Hence, if the fixpoint leads us to conclude that we should
+            // switch back to int, we instead ignore this and stick with double.
             return false;
         }
         
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to