Title: [120137] trunk
Revision
120137
Author
fpi...@apple.com
Date
2012-06-12 16:16:51 -0700 (Tue, 12 Jun 2012)

Log Message

DFG should not ASSERT if you have a double use of a variable that is not revealed to be a double
until after CFG simplification
https://bugs.webkit.org/show_bug.cgi?id=88927
<rdar://problem/11513971>

Source/_javascript_Core: 

Reviewed by Geoffrey Garen.
        
Speculation fixup needs to run if simplification did things, because simplification can change
predictions - particularly if you had a control flow path that stored weird things into a
variable, but that path got axed by the simplifier.
        
Running fixup in the fixpoint requires making it idempotent, which it previously wasn't. Only
one place needed to be changed, namely the un-MustGenerate-ion of ValueToInt32.

* dfg/DFGDriver.cpp:
(JSC::DFG::compile):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):

LayoutTests: 

Reviewed by Geoffrey Garen.

* fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt: Added.
* fast/js/dfg-double-use-of-post-simplification-double-prediction.html: Added.
* fast/js/script-tests/dfg-double-use-of-post-simplification-double-prediction.js: Added.
(foo):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120136 => 120137)


--- trunk/LayoutTests/ChangeLog	2012-06-12 23:07:21 UTC (rev 120136)
+++ trunk/LayoutTests/ChangeLog	2012-06-12 23:16:51 UTC (rev 120137)
@@ -1,3 +1,17 @@
+2012-06-12  Filip Pizlo  <fpi...@apple.com>
+
+        DFG should not ASSERT if you have a double use of a variable that is not revealed to be a double
+        until after CFG simplification
+        https://bugs.webkit.org/show_bug.cgi?id=88927
+        <rdar://problem/11513971>
+
+        Reviewed by Geoffrey Garen.
+
+        * fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt: Added.
+        * fast/js/dfg-double-use-of-post-simplification-double-prediction.html: Added.
+        * fast/js/script-tests/dfg-double-use-of-post-simplification-double-prediction.js: Added.
+        (foo):
+
 2012-06-12  Tony Chang  <t...@chromium.org>
 
         Replaced items in a flexbox should be coerced to display:block

Added: trunk/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt (0 => 120137)


--- trunk/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt	2012-06-12 23:16:51 UTC (rev 120137)
@@ -0,0 +1,209 @@
+Tests stability of the DFG compiler when you have a double use of a variable that is not revealed to be a double until after CFG simplification.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS foo(0.5) is 42.5
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction.html (0 => 120137)


--- trunk/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction.html	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction.html	2012-06-12 23:16:51 UTC (rev 120137)
@@ -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-use-of-post-simplification-double-prediction.js (0 => 120137)


--- trunk/LayoutTests/fast/js/script-tests/dfg-double-use-of-post-simplification-double-prediction.js	                        (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-double-use-of-post-simplification-double-prediction.js	2012-06-12 23:16:51 UTC (rev 120137)
@@ -0,0 +1,17 @@
+description(
+"Tests stability of the DFG compiler when you have a double use of a variable that is not revealed to be a double until after CFG simplification."
+);
+
+function foo(a) {
+    var p = true;
+    var x;
+    if (p)
+        x = 42;
+    else
+        x = "yo";
+    return x + a;
+}
+
+for (var i = 0; i < 200; ++i)
+    shouldBe("foo(0.5)", "42.5");
+

Modified: trunk/Source/_javascript_Core/ChangeLog (120136 => 120137)


--- trunk/Source/_javascript_Core/ChangeLog	2012-06-12 23:07:21 UTC (rev 120136)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-06-12 23:16:51 UTC (rev 120137)
@@ -1,5 +1,26 @@
 2012-06-12  Filip Pizlo  <fpi...@apple.com>
 
+        DFG should not ASSERT if you have a double use of a variable that is not revealed to be a double
+        until after CFG simplification
+        https://bugs.webkit.org/show_bug.cgi?id=88927
+        <rdar://problem/11513971>
+
+        Reviewed by Geoffrey Garen.
+        
+        Speculation fixup needs to run if simplification did things, because simplification can change
+        predictions - particularly if you had a control flow path that stored weird things into a
+        variable, but that path got axed by the simplifier.
+        
+        Running fixup in the fixpoint requires making it idempotent, which it previously wasn't. Only
+        one place needed to be changed, namely the un-MustGenerate-ion of ValueToInt32.
+
+        * dfg/DFGDriver.cpp:
+        (JSC::DFG::compile):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+
+2012-06-12  Filip Pizlo  <fpi...@apple.com>
+
         REGRESSION (r119779): _javascript_ TypeError: 'undefined' is not an object
         https://bugs.webkit.org/show_bug.cgi?id=88783
         <rdar://problem/11640299>

Modified: trunk/Source/_javascript_Core/dfg/DFGDriver.cpp (120136 => 120137)


--- trunk/Source/_javascript_Core/dfg/DFGDriver.cpp	2012-06-12 23:07:21 UTC (rev 120136)
+++ trunk/Source/_javascript_Core/dfg/DFGDriver.cpp	2012-06-12 23:16:51 UTC (rev 120137)
@@ -86,6 +86,7 @@
         if (!changed)
             break;
         dfg.resetExitStates();
+        performFixup(dfg);
     }
     performCSE(dfg, FixpointConverged);
 #if DFG_ENABLE(DEBUG_VERBOSE)

Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (120136 => 120137)


--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2012-06-12 23:07:21 UTC (rev 120136)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2012-06-12 23:16:51 UTC (rev 120137)
@@ -150,7 +150,8 @@
         }
             
         case ValueToInt32: {
-            if (m_graph[node.child1()].shouldSpeculateNumber()) {
+            if (m_graph[node.child1()].shouldSpeculateNumber()
+                && node.mustGenerate()) {
                 node.clearFlags(NodeMustGenerate);
                 m_graph.deref(m_compileIndex);
             }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to