Title: [183419] trunk/Source/_javascript_Core
- Revision
- 183419
- Author
- [email protected]
- Date
- 2015-04-27 13:42:28 -0700 (Mon, 27 Apr 2015)
Log Message
Function allocations shouldn't sink through Put operations
https://bugs.webkit.org/show_bug.cgi?id=144176
Patch by Basile Clement <[email protected]> on 2015-04-27
Reviewed by Filip Pizlo.
By design, we don't support function allocation sinking through any
related operation ; however object allocation can sink through PutByOffset et
al.
Currently, the checks to prevent function allocation to sink through
these are misguided and do not prevent anything ; function allocation sinking
through these operations is prevented as a side effect of requiring an
AllocatePropertyStorage through which the function allocation is seen as
escaping.
This changes it so that ObjectAllocationSinkingPhase::handleNode()
checks properly that only object allocations sink through related write
operations.
* dfg/DFGObjectAllocationSinkingPhase.cpp:
(JSC::DFG::ObjectAllocationSinkingPhase::lowerNonReadingOperationsOnPhantomAllocations):
(JSC::DFG::ObjectAllocationSinkingPhase::handleNode):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (183418 => 183419)
--- trunk/Source/_javascript_Core/ChangeLog 2015-04-27 20:34:33 UTC (rev 183418)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-04-27 20:42:28 UTC (rev 183419)
@@ -1,3 +1,28 @@
+2015-04-27 Basile Clement <[email protected]>
+
+ Function allocations shouldn't sink through Put operations
+ https://bugs.webkit.org/show_bug.cgi?id=144176
+
+ Reviewed by Filip Pizlo.
+
+ By design, we don't support function allocation sinking through any
+ related operation ; however object allocation can sink through PutByOffset et
+ al.
+
+ Currently, the checks to prevent function allocation to sink through
+ these are misguided and do not prevent anything ; function allocation sinking
+ through these operations is prevented as a side effect of requiring an
+ AllocatePropertyStorage through which the function allocation is seen as
+ escaping.
+
+ This changes it so that ObjectAllocationSinkingPhase::handleNode()
+ checks properly that only object allocations sink through related write
+ operations.
+
+ * dfg/DFGObjectAllocationSinkingPhase.cpp:
+ (JSC::DFG::ObjectAllocationSinkingPhase::lowerNonReadingOperationsOnPhantomAllocations):
+ (JSC::DFG::ObjectAllocationSinkingPhase::handleNode):
+
2015-04-25 Filip Pizlo <[email protected]>
VarargsForwardingPhase should use bytecode liveness in addition to other uses to determine the last point that a candidate is used
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (183418 => 183419)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2015-04-27 20:34:33 UTC (rev 183418)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2015-04-27 20:42:28 UTC (rev 183419)
@@ -1420,6 +1420,17 @@
ASSERT(hasObjectMaterializationData());
return *reinterpret_cast<ObjectMaterializationData*>(m_opInfo);
}
+
+ bool isObjectAllocation()
+ {
+ switch (op()) {
+ case NewObject:
+ case MaterializeNewObject:
+ return true;
+ default:
+ return false;
+ }
+ }
bool isPhantomObjectAllocation()
{
Modified: trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (183418 => 183419)
--- trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2015-04-27 20:34:33 UTC (rev 183418)
+++ trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2015-04-27 20:42:28 UTC (rev 183419)
@@ -511,14 +511,17 @@
switch (node->op()) {
case PutByOffset: {
Node* target = node->child2().node();
- if (target->isPhantomObjectAllocation() && m_sinkCandidates.contains(target))
+ if (m_sinkCandidates.contains(target)) {
+ ASSERT(target->isPhantomObjectAllocation());
node->convertToPutByOffsetHint();
+ }
break;
}
case PutStructure: {
Node* target = node->child1().node();
- if (target->isPhantomObjectAllocation() && m_sinkCandidates.contains(target)) {
+ if (m_sinkCandidates.contains(target)) {
+ ASSERT(target->isPhantomObjectAllocation());
Node* structure = m_insertionSet.insertConstant(
nodeIndex, node->origin, JSValue(node->transition()->next));
node->convertToPutStructureHint(structure);
@@ -580,8 +583,10 @@
case StoreBarrier:
case StoreBarrierWithNullCheck: {
Node* target = node->child1().node();
- if (target->isPhantomObjectAllocation() && m_sinkCandidates.contains(target))
+ if (m_sinkCandidates.contains(target)) {
+ ASSERT(target->isPhantomObjectAllocation());
node->convertToPhantom();
+ }
break;
}
@@ -789,22 +794,34 @@
});
break;
+ case MovHint:
+ case Phantom:
+ case Check:
+ case PutHint:
+ break;
+
+ case PutStructure:
case CheckStructure:
case GetByOffset:
case MultiGetByOffset:
- case PutStructure:
case GetGetterSetterByOffset:
- case MovHint:
- case Phantom:
- case Check:
case StoreBarrier:
- case StoreBarrierWithNullCheck:
- case PutHint:
+ case StoreBarrierWithNullCheck: {
+ Node* target = node->child1().node();
+ if (!target->isObjectAllocation())
+ escape(target);
break;
+ }
- case PutByOffset:
+ case PutByOffset: {
+ Node* target = node->child2().node();
+ if (!target->isObjectAllocation()) {
+ escape(target);
+ escape(node->child1().node());
+ }
escape(node->child3().node());
break;
+ }
case MultiPutByOffset:
// FIXME: In the future we should be able to handle this. It's just a matter of
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
