DFG should not ASSERT if you have a double use of a variable that is not revealed...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jun 2012 23:16:51 +0000 (23:16 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jun 2012 23:16:51 +0000 (23:16 +0000)
until after CFG simplification
https://bugs.webkit.org/show_bug.cgi?id=88927
<rdar://problem/11513971>

Source/JavaScriptCore:

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@120137 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction.html [new file with mode: 0644]
LayoutTests/fast/js/script-tests/dfg-double-use-of-post-simplification-double-prediction.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGDriver.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

index e9622f1..3e4fbc4 100644 (file)
@@ -1,3 +1,17 @@
+2012-06-12  Filip Pizlo  <fpizlo@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  <tony@chromium.org>
 
         Replaced items in a flexbox should be coerced to display:block
diff --git a/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt b/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction-expected.txt
new file mode 100644 (file)
index 0000000..26e1fb0
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction.html b/LayoutTests/fast/js/dfg-double-use-of-post-simplification-double-prediction.html
new file mode 100644 (file)
index 0000000..f3ae6e4
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/dfg-double-use-of-post-simplification-double-prediction.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/js/script-tests/dfg-double-use-of-post-simplification-double-prediction.js b/LayoutTests/fast/js/script-tests/dfg-double-use-of-post-simplification-double-prediction.js
new file mode 100644 (file)
index 0000000..82beb27
--- /dev/null
@@ -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");
+
index 28bd08e..b04f2a5 100644 (file)
@@ -1,5 +1,26 @@
 2012-06-12  Filip Pizlo  <fpizlo@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  <fpizlo@apple.com>
+
         REGRESSION (r119779): Javascript TypeError: 'undefined' is not an object
         https://bugs.webkit.org/show_bug.cgi?id=88783
         <rdar://problem/11640299>
index 57461ae..e932792 100644 (file)
@@ -86,6 +86,7 @@ inline bool compile(CompileMode compileMode, ExecState* exec, CodeBlock* codeBlo
         if (!changed)
             break;
         dfg.resetExitStates();
+        performFixup(dfg);
     }
     performCSE(dfg, FixpointConverged);
 #if DFG_ENABLE(DEBUG_VERBOSE)
index b256fac..f6e3c0a 100644 (file)
@@ -150,7 +150,8 @@ private:
         }
             
         case ValueToInt32: {
-            if (m_graph[node.child1()].shouldSpeculateNumber()) {
+            if (m_graph[node.child1()].shouldSpeculateNumber()
+                && node.mustGenerate()) {
                 node.clearFlags(NodeMustGenerate);
                 m_graph.deref(m_compileIndex);
             }