Changeset 171689 in webkit for trunk/Source/JavaScriptCore/dfg

Timestamp:

Jul 28, 2014, 1:41:09 PM (11 years ago)

Author:

[email protected]

Message:

Make sure that we don't use non-speculative BooleanToNumber for a speculative Branch
​https://bugs.webkit.org/show_bug.cgi?id=135350
<rdar://problem/17509889>

Reviewed by Mark Hahnenberg and Oliver Hunt.

If we have an exiting node that uses a conversion node, then that exiting node
needs to have a Phantom after it for the the original node. But we can't do that
for Branch because ​https://bugs.webkit.org/show_bug.cgi?id=126778.

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::clearPhantomsAtEnd):

• tests/stress/branch-check-int32-on-boolean-to-number-untyped.js: Added.

(foo):
(test):

• tests/stress/branch-check-number-on-boolean-to-number-untyped.js: Added.

(foo):
(test):

File:

• trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -709 +709 @@
             else if (node->child1()->shouldSpeculateObjectOrOther())
                 fixEdge<ObjectOrOtherUse>(node->child1());
-            else if (node->child1()->shouldSpeculateInt32OrBoolean())
-                fixIntOrBooleanEdge(node->child1());
-            else if (node->child1()->shouldSpeculateNumberOrBoolean())
-                fixDoubleOrBooleanEdge(node->child1());
+            // FIXME: We should just be able to do shouldSpeculateInt32OrBoolean() and
+            // shouldSpeculateNumberOrBoolean() here, but we can't because then the Branch
+            // could speculate on the result of a non-speculative conversion node.
+            // https://bugs.webkit.org/show_bug.cgi?id=126778
+            else if (node->child1()->shouldSpeculateInt32())
+                fixEdge<Int32Use>(node->child1());
+            else if (node->child1()->shouldSpeculateNumber())
+                fixEdge<DoubleRepUse>(node->child1());
             break;
         }
@@ -1982 +1986 @@
         // Terminal nodes don't need post-phantoms, and inserting them would violate
         // the current requirement that a terminal is the last thing in a block. We
-        // should eventually change that requirement but even if we did, this would
-        // still be a valid optimization. All terminals accept just one input, and
-        // if that input is a conversion node then no further speculations will be
-        // performed.
-        
+        // should eventually change that requirement. Currently we get around this by
+        // ensuring that all terminals accept just one input, and if that input is a
+        // conversion node then no further speculations will be performed. See
+        // references to the bug, below, for places where we have to have hacks to
+        // work around this.
         // FIXME: Get rid of this by allowing Phantoms after terminals.
         // https://bugs.webkit.org/show_bug.cgi?id=126778