Title: [144481] trunk/Source/_javascript_Core
- Revision
- 144481
- Author
- [email protected]
- Date
- 2013-03-01 12:40:05 -0800 (Fri, 01 Mar 2013)
Log Message
DFG CSE phase shouldn't rely on ref count of nodes, since it doesn't have to
https://bugs.webkit.org/show_bug.cgi?id=111205
Reviewed by Oliver Hunt.
I don't understand the intuition behind setLocalStoreElimination() validating that the SetLocal's ref count
is 1. I believe this is a hold-over from when setLocalStoreElimination() would match one SetLocal to another,
and then try to eliminate the first SetLocal. But that's not how it works now. Now, setLocalStoreElimination()
is actually Flush elimination: it eliminates any Flush that anchors a SetLocal if it proves that every path
from the SetLocal to the Flush is devoid of operations that may observe the local. It doesn't actually kill
the SetLocal itself: if the SetLocal is live because of other things (other Flushes or GetLocals in other
basic blocks), then the SetLocal will naturally still be alive because th Flush was only keeping the SetLocal
alive by one count rather than being solely responsible for its liveness.
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::setLocalStoreElimination):
(JSC::DFG::CSEPhase::eliminate):
(JSC::DFG::CSEPhase::performNodeCSE):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (144480 => 144481)
--- trunk/Source/_javascript_Core/ChangeLog 2013-03-01 20:32:24 UTC (rev 144480)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-03-01 20:40:05 UTC (rev 144481)
@@ -1,5 +1,26 @@
2013-03-01 Filip Pizlo <[email protected]>
+ DFG CSE phase shouldn't rely on ref count of nodes, since it doesn't have to
+ https://bugs.webkit.org/show_bug.cgi?id=111205
+
+ Reviewed by Oliver Hunt.
+
+ I don't understand the intuition behind setLocalStoreElimination() validating that the SetLocal's ref count
+ is 1. I believe this is a hold-over from when setLocalStoreElimination() would match one SetLocal to another,
+ and then try to eliminate the first SetLocal. But that's not how it works now. Now, setLocalStoreElimination()
+ is actually Flush elimination: it eliminates any Flush that anchors a SetLocal if it proves that every path
+ from the SetLocal to the Flush is devoid of operations that may observe the local. It doesn't actually kill
+ the SetLocal itself: if the SetLocal is live because of other things (other Flushes or GetLocals in other
+ basic blocks), then the SetLocal will naturally still be alive because th Flush was only keeping the SetLocal
+ alive by one count rather than being solely responsible for its liveness.
+
+ * dfg/DFGCSEPhase.cpp:
+ (JSC::DFG::CSEPhase::setLocalStoreElimination):
+ (JSC::DFG::CSEPhase::eliminate):
+ (JSC::DFG::CSEPhase::performNodeCSE):
+
+2013-03-01 Filip Pizlo <[email protected]>
+
Rename MovHint to MovHintEvent so I can create a NodeType called MovHint
Rubber stamped by Mark Hahnenberg.
Modified: trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (144480 => 144481)
--- trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-03-01 20:32:24 UTC (rev 144480)
+++ trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-03-01 20:40:05 UTC (rev 144481)
@@ -939,8 +939,6 @@
break;
if (node != expectedNode)
result.mayBeAccessed = true;
- if (node->refCount() > 1)
- result.mayBeAccessed = true;
return result;
}
@@ -1043,7 +1041,6 @@
dataLogF(" Eliminating @%u", m_currentNode->index());
#endif
- ASSERT(m_currentNode->refCount() == 1);
ASSERT(m_currentNode->mustGenerate());
m_currentNode->convertToPhantom();
eliminateIrrelevantPhantomChildren(m_currentNode);
@@ -1055,8 +1052,6 @@
{
if (!node)
return;
- if (node->refCount() != 1)
- return;
ASSERT(node->mustGenerate());
node->setOpAndDefaultNonExitFlags(phantomType);
if (phantomType == Phantom)
@@ -1208,7 +1203,6 @@
if (result.mayBeAccessed || result.mayClobberWorld)
break;
ASSERT(replacement->op() == SetLocal);
- ASSERT(replacement->refCount() == 1);
ASSERT(replacement->shouldGenerate());
// FIXME: Investigate using mayExit as a further optimization.
node->convertToPhantom();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
