Modified: trunk/Source/_javascript_Core/ChangeLog (200497 => 200498)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-05 23:59:52 UTC (rev 200497)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-06 00:05:04 UTC (rev 200498)
@@ -1,3 +1,35 @@
+2016-05-05 Benjamin Poulain <[email protected]>
+
+ [JSC] In DFG, an OSR Exit on SetLocal can trash its child node
+ https://bugs.webkit.org/show_bug.cgi?id=157358
+ rdar://problem/25339647
+
+ Reviewed by Filip Pizlo.
+
+ When we OSR Exit on SetLocal, the child is never restored if its representation
+ was changed since the MovHint.
+
+ For example, say we have:
+ @1 = SomethingProducingDouble()
+ @2 = MovHint(@1)
+ @3 = ValueRep(@1)
+ @4 = SetLocal(@3, FlushedInt32)
+
+ When we lower SetLocal(), we start by speculating that @3 is an Int32.
+ Now this can fail if @1 was really a double.
+ When that happens, we go over the VariableEventStream to find where values
+ are, and @1 died at @3. Since the speculation failure happens before
+ the SetLocal event, we don't do anything with @3.
+
+ In this patch, I extend the PhantomInsertion phase to keep the MovHint
+ alive past the SetLocal.
+
+ * dfg/DFGPhantomInsertionPhase.cpp:
+ * tests/stress/multiply-typed-double-and-object.js: Added.
+ (otherObject.valueOf):
+ (targetDFG.multiply):
+ (targetFTL.multiply):
+
2016-05-05 Oliver Hunt <[email protected]>
Enable separated heap by default on ios
Modified: trunk/Source/_javascript_Core/dfg/DFGPhantomInsertionPhase.cpp (200497 => 200498)
--- trunk/Source/_javascript_Core/dfg/DFGPhantomInsertionPhase.cpp 2016-05-05 23:59:52 UTC (rev 200497)
+++ trunk/Source/_javascript_Core/dfg/DFGPhantomInsertionPhase.cpp 2016-05-06 00:05:04 UTC (rev 200498)
@@ -113,7 +113,6 @@
m_values.operand(node->unlinkedLocal()) = nullptr;
break;
- case SetLocal:
case GetLocal:
case SetArgument:
m_values.operand(node->local()) = nullptr;
@@ -122,8 +121,9 @@
default:
break;
}
-
- if (mayExit(m_graph, node) != DoesNotExit) {
+
+ bool nodeMayExit = mayExit(m_graph, node) != DoesNotExit;
+ if (nodeMayExit) {
currentEpoch.bump();
lastExitingIndex = nodeIndex;
}
@@ -136,38 +136,53 @@
node->setEpoch(currentEpoch);
- forAllKilledOperands(
- m_graph, node, block->tryAt(nodeIndex + 1),
- [&] (VirtualRegister reg) {
- if (verbose)
- dataLog(" Killed operand: ", reg, "\n");
-
- Node* killedNode = m_values.operand(reg);
- if (!killedNode)
- return;
-
- // We only need to insert a Phantom if the node hasn't been used since the last
- // exit, and was born before the last exit.
- if (killedNode->epoch() == currentEpoch)
- return;
-
- if (verbose) {
- dataLog(
- " Inserting Phantom on ", killedNode, " after ",
- block->at(lastExitingIndex), "\n");
- }
-
- // We have exact ref counts, so creating a new use means that we have to
- // increment the ref count.
- killedNode->postfixRef();
+ VirtualRegister alreadyKilled;
- Node* lastExitingNode = block->at(lastExitingIndex);
-
- m_insertionSet.insertNode(
- lastExitingIndex + 1, SpecNone, Phantom,
- lastExitingNode->origin.forInsertingAfter(m_graph, lastExitingNode),
- killedNode->defaultEdge());
- });
+ auto processKilledOperand = [&] (VirtualRegister reg) {
+ if (verbose)
+ dataLog(" Killed operand: ", reg, "\n");
+
+ // Already handled from SetLocal.
+ if (reg == alreadyKilled)
+ return;
+
+ Node* killedNode = m_values.operand(reg);
+ if (!killedNode)
+ return;
+
+ // We only need to insert a Phantom if the node hasn't been used since the last
+ // exit, and was born before the last exit.
+ if (killedNode->epoch() == currentEpoch)
+ return;
+
+ if (verbose) {
+ dataLog(
+ " Inserting Phantom on ", killedNode, " after ",
+ block->at(lastExitingIndex), "\n");
+ }
+
+ // We have exact ref counts, so creating a new use means that we have to
+ // increment the ref count.
+ killedNode->postfixRef();
+
+ Node* lastExitingNode = block->at(lastExitingIndex);
+
+ m_insertionSet.insertNode(
+ lastExitingIndex + 1, SpecNone, Phantom,
+ lastExitingNode->origin.forInsertingAfter(m_graph, lastExitingNode),
+ killedNode->defaultEdge());
+ };
+
+ if (nodeMayExit && node->op() == SetLocal) {
+ // If the SetLocal does exit, we need the MovHint of its local
+ // to be live until the SetLocal is done.
+ VirtualRegister local = node->local();
+ processKilledOperand(local);
+ alreadyKilled = local;
+ m_values.operand(local) = nullptr;
+ }
+
+ forAllKilledOperands(m_graph, node, block->tryAt(nodeIndex + 1), processKilledOperand);
}
m_insertionSet.execute(block);
Added: trunk/Source/_javascript_Core/tests/stress/multiply-typed-double-and-object.js (0 => 200498)
--- trunk/Source/_javascript_Core/tests/stress/multiply-typed-double-and-object.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/multiply-typed-double-and-object.js 2016-05-06 00:05:04 UTC (rev 200498)
@@ -0,0 +1,63 @@
+var otherObject = {
+ valueOf: function() { return 5.1; }
+};
+// DFG.
+var targetDFG = {
+ value: 5.5,
+ multiply: function(arg) {
+ let returnValue = 1;
+ if (typeof arg == "number") {
+ returnValue = this.value * arg;
+ }
+ return returnValue + 1;
+ }
+};
+noInline(targetDFG.multiply);
+
+for (let i = 0; i < 400; ++i) {
+ if (targetDFG.multiply(otherObject) !== 2)
+ throw "Failed targetDFG.multiply(otherObject)";
+ let result = targetDFG.multiply(Math.PI);
+ if (result !== (5.5 * Math.PI + 1))
+ throw "Failed targetDFG.multiply(Math.PI), expected " + (5.5 * Math.PI + 1) + " got " + result + " at iteration " + i;
+}
+for (let i = 0; i < 1e3; ++i) {
+ let result = targetDFG.multiply(Math.PI);
+ if (result !== (5.5 * Math.PI + 1))
+ throw "Failed targetDFG.multiply(Math.PI), expected " + (5.5 * Math.PI + 1) + " got " + result + " at iteration " + i;
+}
+
+// FTL.
+var targetFTL = {
+ value: 5.5,
+ multiply: function(arg) {
+ let returnValue = 1;
+ if (typeof arg == "number") {
+ returnValue = this.value * arg;
+ }
+ return returnValue + 1;
+ }
+};
+noInline(targetFTL.multiply);
+
+// Warmup to baseline.
+for (let i = 0; i < 400; ++i) {
+ if (targetFTL.multiply(otherObject) !== 2)
+ throw "Failed targetFTL.multiply(otherObject)";
+ let result = targetFTL.multiply(Math.PI);
+ if (result !== (5.5 * Math.PI + 1))
+ throw "Failed targetFTL.multiply(Math.PI), expected " + (5.5 * Math.PI + 1) + " got " + result + " at iteration " + i;
+}
+
+// Step over DFG *WITHOUT* OSR Exit.
+for (let i = 0; i < 1e6; ++i) {
+ if (targetFTL.multiply(otherObject) !== 2)
+ throw "Failed targetFTL.multiply(otherObject)";
+}
+
+// Now OSR Exit in FTL.
+for (let i = 0; i < 1e2; ++i) {
+ let result = targetFTL.multiply(Math.PI);
+ if (result !== (5.5 * Math.PI + 1))
+ throw "Failed targetFTL.multiply(Math.PI), expected " + (5.5 * Math.PI + 1) + " got " + result + " at iteration " + i;
+}
