Title: [171689] trunk/Source/_javascript_Core
- Revision
- 171689
- Author
- fpi...@apple.com
- Date
- 2014-07-28 13:41:09 -0700 (Mon, 28 Jul 2014)
Log 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):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (171688 => 171689)
--- trunk/Source/_javascript_Core/ChangeLog 2014-07-28 20:38:32 UTC (rev 171688)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-07-28 20:41:09 UTC (rev 171689)
@@ -1,3 +1,25 @@
+2014-07-28 Filip Pizlo <fpi...@apple.com>
+
+ 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):
+
2014-07-28 Joseph Pecoraro <pecor...@apple.com>
JSContext Inspector: crash when using step-into
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (171688 => 171689)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2014-07-28 20:38:32 UTC (rev 171688)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2014-07-28 20:41:09 UTC (rev 171689)
@@ -708,10 +708,14 @@
fixEdge<BooleanUse>(node->child1());
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;
}
@@ -1981,11 +1985,11 @@
{
// 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
Added: trunk/Source/_javascript_Core/tests/stress/branch-check-int32-on-boolean-to-number-untyped.js (0 => 171689)
--- trunk/Source/_javascript_Core/tests/stress/branch-check-int32-on-boolean-to-number-untyped.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/branch-check-int32-on-boolean-to-number-untyped.js 2014-07-28 20:41:09 UTC (rev 171689)
@@ -0,0 +1,23 @@
+function foo(o) {
+ if (o.f)
+ return "yes";
+ else
+ return "no";
+}
+
+noInline(foo);
+
+function test(value, expected) {
+ var result = foo({f:value});
+ if (result != expected)
+ throw "Error: bad result for " + value + ": " + result;
+}
+
+for (var i = 0; i < 10000; ++i) {
+ test(1, "yes");
+ test(0, "no");
+ test(true, "yes");
+ test(false, "no");
+}
+
+test("yes", "yes");
Added: trunk/Source/_javascript_Core/tests/stress/branch-check-number-on-boolean-to-number-untyped.js (0 => 171689)
--- trunk/Source/_javascript_Core/tests/stress/branch-check-number-on-boolean-to-number-untyped.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/branch-check-number-on-boolean-to-number-untyped.js 2014-07-28 20:41:09 UTC (rev 171689)
@@ -0,0 +1,23 @@
+function foo(o) {
+ if (o.f)
+ return "yes";
+ else
+ return "no";
+}
+
+noInline(foo);
+
+function test(value, expected) {
+ var result = foo({f:value});
+ if (result != expected)
+ throw "Error: bad result for " + value + ": " + result;
+}
+
+for (var i = 0; i < 10000; ++i) {
+ test(1.5, "yes");
+ test(0.0, "no");
+ test(true, "yes");
+ test(false, "no");
+}
+
+test("yes", "yes");
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
