Diff
Modified: branches/jsCStack/Source/_javascript_Core/ChangeLog (161682 => 161683)
--- branches/jsCStack/Source/_javascript_Core/ChangeLog 2014-01-10 22:54:16 UTC (rev 161682)
+++ branches/jsCStack/Source/_javascript_Core/ChangeLog 2014-01-10 23:17:57 UTC (rev 161683)
@@ -1,3 +1,30 @@
+2014-01-10 Filip Pizlo <[email protected]>
+
+ DFG should insert Phantoms when it uses conversion nodes
+ https://bugs.webkit.org/show_bug.cgi?id=126777
+
+ Reviewed by Oliver Hunt.
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::fixupSetLocalsInBlock):
+ (JSC::DFG::FixupPhase::fixupUntypedSetLocalsInBlock):
+ (JSC::DFG::FixupPhase::fixEdge):
+ (JSC::DFG::FixupPhase::fixIntEdge):
+ (JSC::DFG::FixupPhase::injectInt32ToDoubleNode):
+ (JSC::DFG::FixupPhase::addPhantomsIfNecessary):
+ * dfg/DFGNodeFlags.cpp:
+ (JSC::DFG::dumpNodeFlags):
+ * dfg/DFGValidate.cpp:
+ (JSC::DFG::Validate::validate):
+ * tests/stress/exit-after-int32-to-double.js: Added.
+ (foo):
+ * tests/stress/exit-after-int52-to-double.js: Added.
+ (foo):
+ * tests/stress/exit-after-int52-to-value.js: Added.
+ (foo):
+ (makeWeirdObject):
+
2014-01-09 Mark Lam <[email protected]>
Merge trunk r161446.
Modified: branches/jsCStack/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp (161682 => 161683)
--- branches/jsCStack/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp 2014-01-10 22:54:16 UTC (rev 161682)
+++ branches/jsCStack/Source/_javascript_Core/dfg/DFGCFGSimplificationPhase.cpp 2014-01-10 23:17:57 UTC (rev 161683)
@@ -79,6 +79,8 @@
// suboptimal, because if my successor has multiple predecessors then we'll
// be keeping alive things on other predecessor edges unnecessarily.
// What we really need is the notion of end-of-block ghosties!
+ // FIXME: Allow putting phantoms after terminals.
+ // https://bugs.webkit.org/show_bug.cgi?id=126778
break;
}
Modified: branches/jsCStack/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (161682 => 161683)
--- branches/jsCStack/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2014-01-10 22:54:16 UTC (rev 161682)
+++ branches/jsCStack/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2014-01-10 23:17:57 UTC (rev 161683)
@@ -100,12 +100,14 @@
case BitRShift:
case BitLShift:
case BitURShift: {
- fixBinaryIntEdges();
+ fixIntEdge(node->child1());
+ fixIntEdge(node->child2());
break;
}
case ArithIMul: {
- fixBinaryIntEdges();
+ fixIntEdge(node->child1());
+ fixIntEdge(node->child2());
node->setOp(ArithMul);
node->setArithMode(Arith::Unchecked);
node->child1().setUseKind(Int32Use);
@@ -980,6 +982,19 @@
if (!node->containsMovHint())
DFG_NODE_DO_TO_CHILDREN(m_graph, node, observeUntypedEdge);
+
+ if (node->isTerminal()) {
+ // 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.
+ // FIXME: Get rid of this by allowing Phantoms after terminals.
+ // https://bugs.webkit.org/show_bug.cgi?id=126778
+ m_requiredPhantoms.resize(0);
+ } else
+ addPhantomsIfNecessary();
}
void observeUntypedEdge(Node*, Edge& edge)
@@ -1265,6 +1280,7 @@
RELEASE_ASSERT_NOT_REACHED();
break;
}
+ addPhantomsIfNecessary();
}
m_insertionSet.execute(block);
}
@@ -1280,8 +1296,10 @@
if (node->op() != SetLocal)
continue;
- if (node->child1().useKind() == UntypedUse)
+ if (node->child1().useKind() == UntypedUse) {
fixEdge<UntypedUse>(node->child1());
+ addPhantomsIfNecessary();
+ }
}
m_insertionSet.execute(block);
}
@@ -1455,6 +1473,7 @@
// Int8ToDouble will convert int52's that fit in an int32 into a double
// rather than trying to create a boxed int32 like Int52ToValue does.
+ m_requiredPhantoms.append(edge.node());
Node* result = m_insertionSet.insertNode(
m_indexInBlock, SpecInt52AsDouble, Int52ToDouble,
m_currentNode->codeOrigin, Edge(edge.node(), NumberUse));
@@ -1508,6 +1527,7 @@
//
// But the solution we use handles the above gracefully.
+ m_requiredPhantoms.append(edge.node());
Node* result = m_insertionSet.insertNode(
m_indexInBlock, SpecInt52, Int52ToValue,
m_currentNode->codeOrigin, Edge(edge.node(), UntypedUse));
@@ -1520,12 +1540,12 @@
edge.setUseKind(useKind);
}
- bool fixIntEdge(Edge& edge)
+ void fixIntEdge(Edge& edge)
{
Node* node = edge.node();
if (node->shouldSpeculateInt32()) {
fixEdge<Int32Use>(edge);
- return false;
+ return;
}
UseKind useKind;
@@ -1543,24 +1563,13 @@
observeUseKindOnNode(node, useKind);
edge = Edge(newNode, KnownInt32Use);
- return true;
+ m_requiredPhantoms.append(node);
}
- void fixBinaryIntEdges()
+ void injectInt32ToDoubleNode(Edge& edge, UseKind useKind = NumberUse)
{
- AdjacencyList children = m_currentNode->children;
+ m_requiredPhantoms.append(edge.node());
- // Call fixIntEdge() on both edges.
- bool needPhantom =
- fixIntEdge(m_currentNode->child1()) | fixIntEdge(m_currentNode->child2());
-
- if (!needPhantom)
- return;
- m_insertionSet.insertNode(m_indexInBlock + 1, SpecNone, Phantom, m_currentNode->codeOrigin, children);
- }
-
- void injectInt32ToDoubleNode(Edge& edge, UseKind useKind = NumberUse)
- {
Node* result = m_insertionSet.insertNode(
m_indexInBlock, SpecInt52AsDouble, Int32ToDouble,
m_currentNode->codeOrigin, Edge(edge.node(), NumberUse));
@@ -1746,12 +1755,27 @@
fixEdge<KnownCellUse>(node->child1());
return true;
}
+
+ void addPhantomsIfNecessary()
+ {
+ if (m_requiredPhantoms.isEmpty())
+ return;
+
+ for (unsigned i = m_requiredPhantoms.size(); i--;) {
+ m_insertionSet.insertNode(
+ m_indexInBlock + 1, SpecNone, Phantom, m_currentNode->codeOrigin,
+ Edge(m_requiredPhantoms[i], UntypedUse));
+ }
+
+ m_requiredPhantoms.resize(0);
+ }
BasicBlock* m_block;
unsigned m_indexInBlock;
Node* m_currentNode;
InsertionSet m_insertionSet;
bool m_profitabilityChanged;
+ Vector<Node*, 3> m_requiredPhantoms;
};
bool performFixup(Graph& graph)
Modified: branches/jsCStack/Source/_javascript_Core/dfg/DFGNodeFlags.cpp (161682 => 161683)
--- branches/jsCStack/Source/_javascript_Core/dfg/DFGNodeFlags.cpp 2014-01-10 22:54:16 UTC (rev 161682)
+++ branches/jsCStack/Source/_javascript_Core/dfg/DFGNodeFlags.cpp 2014-01-10 23:17:57 UTC (rev 161683)
@@ -49,6 +49,9 @@
case NodeResultInt32:
out.print(comma, "Int32");
break;
+ case NodeResultInt52:
+ out.print(comma, "Int52");
+ break;
case NodeResultBoolean:
out.print(comma, "Boolean");
break;
Modified: branches/jsCStack/Source/_javascript_Core/dfg/DFGValidate.cpp (161682 => 161683)
--- branches/jsCStack/Source/_javascript_Core/dfg/DFGValidate.cpp 2014-01-10 22:54:16 UTC (rev 161682)
+++ branches/jsCStack/Source/_javascript_Core/dfg/DFGValidate.cpp 2014-01-10 23:17:57 UTC (rev 161683)
@@ -182,6 +182,11 @@
else
V_EQUAL((node), node->refCount(), 1);
}
+
+ for (size_t i = 0 ; i < block->size() - 1; ++i) {
+ Node* node = block->at(i);
+ VALIDATE((node), !node->isTerminal());
+ }
}
switch (m_graph.m_form) {
Added: branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int32-to-double.js (0 => 161683)
--- branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int32-to-double.js (rev 0)
+++ branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int32-to-double.js 2014-01-10 23:17:57 UTC (rev 161683)
@@ -0,0 +1,15 @@
+function foo(x, o) {
+ return o.f + x;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+ var result = foo(42.5, {f:5});
+ if (result != 47.5)
+ throw "Error: bad result: " + result;
+}
+
+var result = foo("42", {f:5});
+if (result != "542")
+ throw "Error: bad result: " + result;
Added: branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int52-to-double.js (0 => 161683)
--- branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int52-to-double.js (rev 0)
+++ branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int52-to-double.js 2014-01-10 23:17:57 UTC (rev 161683)
@@ -0,0 +1,16 @@
+function foo(a, b, c) {
+ return a.f + b.f + c.f;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+ var result = foo({f:2000000000}, {f:2000000000}, {f:0.5});
+ if (result != 4000000000.5)
+ throw "Error: bad result: " + result;
+}
+
+var result = foo({f:2000000000}, {f:2000000000}, {f:"42"});
+if (result != "400000000042")
+ throw "Error: bad result: " + result;
+
Added: branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int52-to-value.js (0 => 161683)
--- branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int52-to-value.js (rev 0)
+++ branches/jsCStack/Source/_javascript_Core/tests/stress/exit-after-int52-to-value.js 2014-01-10 23:17:57 UTC (rev 161683)
@@ -0,0 +1,25 @@
+function foo(a, b, c) {
+ c.f.f = a.f + b.f;
+}
+
+noInline(foo);
+
+var counter = 0;
+function makeWeirdObject() {
+ var result = {};
+ result["blah" + (counter++)] = 42;
+ return result;
+}
+
+for (var i = 0; i < 100000; ++i) {
+ var o = makeWeirdObject();
+ foo({f:2000000000}, {f:2000000000}, {f:o});
+ if (o.f != 4000000000)
+ throw "Error: bad result: " + result;
+}
+
+var thingy;
+Number.prototype.__defineSetter__("f", function(value) { thingy = value; });
+foo({f:2000000000}, {f:2000000000}, {f:42});
+if (thingy != 4000000000)
+ throw "Error: bad result: " + thingy;
