Title: [184755] trunk/Source/_javascript_Core
- Revision
- 184755
- Author
- fpi...@apple.com
- Date
- 2015-05-21 23:32:30 -0700 (Thu, 21 May 2015)
Log Message
CPS rethreading should really get rid of GetLocals
https://bugs.webkit.org/show_bug.cgi?id=145290
Reviewed by Benjamin Poulain.
CPS rethreading is intended to get rid of redundant GetLocals. CSE can also do it, but
the idea is that you should be able to disable CSE and everything would still work. This
fixes a bug in CPS rethreading's GetLocal elimination: we should be calling replaceWith
rather than setReplacement, since setReplacement still leaves the original node.
* dfg/DFGCPSRethreadingPhase.cpp:
(JSC::DFG::CPSRethreadingPhase::canonicalizeGetLocalFor): Fix the bug.
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode): Eliminating GetLocals means that they turn into Check. We should handle Checks that have zero inputs.
* dfg/DFGValidate.cpp:
(JSC::DFG::Validate::validateCPS): Add a validation for what a GetLocal should look like in ThreadedCPS.
* tests/stress/get-local-elimination.js: Added.
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (184754 => 184755)
--- trunk/Source/_javascript_Core/ChangeLog 2015-05-22 06:12:05 UTC (rev 184754)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-05-22 06:32:30 UTC (rev 184755)
@@ -1,3 +1,24 @@
+2015-05-21 Filip Pizlo <fpi...@apple.com>
+
+ CPS rethreading should really get rid of GetLocals
+ https://bugs.webkit.org/show_bug.cgi?id=145290
+
+ Reviewed by Benjamin Poulain.
+
+ CPS rethreading is intended to get rid of redundant GetLocals. CSE can also do it, but
+ the idea is that you should be able to disable CSE and everything would still work. This
+ fixes a bug in CPS rethreading's GetLocal elimination: we should be calling replaceWith
+ rather than setReplacement, since setReplacement still leaves the original node.
+
+ * dfg/DFGCPSRethreadingPhase.cpp:
+ (JSC::DFG::CPSRethreadingPhase::canonicalizeGetLocalFor): Fix the bug.
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode): Eliminating GetLocals means that they turn into Check. We should handle Checks that have zero inputs.
+ * dfg/DFGValidate.cpp:
+ (JSC::DFG::Validate::validateCPS): Add a validation for what a GetLocal should look like in ThreadedCPS.
+ * tests/stress/get-local-elimination.js: Added.
+ (foo):
+
2015-05-21 Saam Barati <saambara...@gmail.com>
Object allocation sinking phase should explicitly create bottom values for CreateActivation sink candidates and CreateActivation should have SymbolTable as a child node
Modified: trunk/Source/_javascript_Core/dfg/DFGCPSRethreadingPhase.cpp (184754 => 184755)
--- trunk/Source/_javascript_Core/dfg/DFGCPSRethreadingPhase.cpp 2015-05-22 06:12:05 UTC (rev 184754)
+++ trunk/Source/_javascript_Core/dfg/DFGCPSRethreadingPhase.cpp 2015-05-22 06:32:30 UTC (rev 184755)
@@ -189,12 +189,12 @@
if (otherNode->op() == GetLocal) {
// Replace all references to this GetLocal with otherNode.
- node->setReplacement(otherNode);
+ node->replaceWith(otherNode);
return;
}
ASSERT(otherNode->op() == SetLocal);
- node->setReplacement(otherNode->child1().node());
+ node->replaceWith(otherNode->child1().node());
return;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (184754 => 184755)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2015-05-22 06:12:05 UTC (rev 184754)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2015-05-22 06:32:30 UTC (rev 184755)
@@ -1050,15 +1050,19 @@
}
case Check: {
- switch (node->child1().useKind()) {
- case NumberUse:
- if (node->child1()->shouldSpeculateInt32ForArithmetic())
- node->child1().setUseKind(Int32Use);
- break;
- default:
- break;
- }
- observeUseKindOnEdge(node->child1());
+ m_graph.doToChildren(
+ node,
+ [&] (Edge& edge) {
+ switch (edge.useKind()) {
+ case NumberUse:
+ if (edge->shouldSpeculateInt32ForArithmetic())
+ edge.setUseKind(Int32Use);
+ break;
+ default:
+ break;
+ }
+ observeUseKindOnEdge(edge);
+ });
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGValidate.cpp (184754 => 184755)
--- trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2015-05-22 06:12:05 UTC (rev 184754)
+++ trunk/Source/_javascript_Core/dfg/DFGValidate.cpp 2015-05-22 06:32:30 UTC (rev 184755)
@@ -452,8 +452,10 @@
// doesn't yet know to be dead.
if (!m_myRefCounts.get(node))
break;
- if (m_graph.m_form == ThreadedCPS)
+ if (m_graph.m_form == ThreadedCPS) {
VALIDATE((node, block), getLocalPositions.operand(node->local()) == notSet);
+ VALIDATE((node, block), !!node->child1());
+ }
getLocalPositions.operand(node->local()) = i;
break;
case SetLocal:
Added: trunk/Source/_javascript_Core/tests/stress/get-local-elimination.js (0 => 184755)
--- trunk/Source/_javascript_Core/tests/stress/get-local-elimination.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/get-local-elimination.js 2015-05-22 06:32:30 UTC (rev 184755)
@@ -0,0 +1,15 @@
+var True = true;
+
+function foo(a) {
+ var x = a;
+ if (True)
+ return a + x;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo(42);
+ if (result != 84)
+ throw "Error: bad result: " + result;
+}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
