Title: [209595] trunk/Source/_javascript_Core
- Revision
- 209595
- Author
- [email protected]
- Date
- 2016-12-08 20:53:33 -0800 (Thu, 08 Dec 2016)
Log Message
MultiPutByOffset should get a barrier if it transitions
https://bugs.webkit.org/show_bug.cgi?id=165646
Reviewed by Keith Miller.
Previously, if we knew that we were storing a non-cell but we needed to transition, we
would fail to add the barrier but the FTL's lowering expected the barrier to be there.
Strictly, we need to "consider" the barrier on MultiPutByOffset if the value is
possibly a cell or if the MultiPutByOffset may transition. Then "considering" the
barrier implies checking if the base is possibly old.
But because the barrier is so cheap anyway, this patch implements something safer: we
just consider the barrier on MultiPutByOffset unconditionally, which opts it out of any
barrier optimizations other than those based on the predicted state of the base. Those
optimizations are already sound - for example they use doesGC() to detect safepoints
and that function correctly predicts when MultiPutByOffset could GC.
Because the barrier optimizations are only a very small speed-up, I think it's great to
fix bugs by weakening the optimizer without cleverness.
* dfg/DFGFixupPhase.cpp:
* dfg/DFGStoreBarrierInsertionPhase.cpp:
* heap/MarkedBlock.cpp:
(JSC::MarkedBlock::assertValidCell):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (209594 => 209595)
--- trunk/Source/_javascript_Core/ChangeLog 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-12-09 04:53:33 UTC (rev 209595)
@@ -1,5 +1,33 @@
2016-12-08 Filip Pizlo <[email protected]>
+ MultiPutByOffset should get a barrier if it transitions
+ https://bugs.webkit.org/show_bug.cgi?id=165646
+
+ Reviewed by Keith Miller.
+
+ Previously, if we knew that we were storing a non-cell but we needed to transition, we
+ would fail to add the barrier but the FTL's lowering expected the barrier to be there.
+
+ Strictly, we need to "consider" the barrier on MultiPutByOffset if the value is
+ possibly a cell or if the MultiPutByOffset may transition. Then "considering" the
+ barrier implies checking if the base is possibly old.
+
+ But because the barrier is so cheap anyway, this patch implements something safer: we
+ just consider the barrier on MultiPutByOffset unconditionally, which opts it out of any
+ barrier optimizations other than those based on the predicted state of the base. Those
+ optimizations are already sound - for example they use doesGC() to detect safepoints
+ and that function correctly predicts when MultiPutByOffset could GC.
+
+ Because the barrier optimizations are only a very small speed-up, I think it's great to
+ fix bugs by weakening the optimizer without cleverness.
+
+ * dfg/DFGFixupPhase.cpp:
+ * dfg/DFGStoreBarrierInsertionPhase.cpp:
+ * heap/MarkedBlock.cpp:
+ (JSC::MarkedBlock::assertValidCell):
+
+2016-12-08 Filip Pizlo <[email protected]>
+
Enable concurrent GC on ARM64
https://bugs.webkit.org/show_bug.cgi?id=165643
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (209594 => 209595)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2016-12-09 04:53:33 UTC (rev 209595)
@@ -1335,7 +1335,6 @@
case MultiPutByOffset: {
fixEdge<CellUse>(node->child1());
- speculateForBarrier(node->child2());
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp (209594 => 209595)
--- trunk/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp 2016-12-09 04:53:33 UTC (rev 209595)
@@ -272,12 +272,16 @@
case PutClosureVar:
case PutToArguments:
- case SetRegExpObjectLastIndex:
- case MultiPutByOffset: {
+ case SetRegExpObjectLastIndex: {
considerBarrier(m_node->child1(), m_node->child2());
break;
}
+ case MultiPutByOffset: {
+ considerBarrier(m_node->child1());
+ break;
+ }
+
case PutByOffset: {
considerBarrier(m_node->child2(), m_node->child3());
break;
@@ -325,7 +329,6 @@
case AllocatePropertyStorage:
case ReallocatePropertyStorage:
- // These allocate but then run their own barrier.
insertBarrier(m_nodeIndex + 1, m_node->child1());
m_node->setEpoch(Epoch());
break;
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.cpp (209594 => 209595)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.cpp 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.cpp 2016-12-09 04:53:33 UTC (rev 209595)
@@ -568,8 +568,8 @@
#if !ASSERT_DISABLED
void MarkedBlock::assertValidCell(VM& vm, HeapCell* cell) const
{
- ASSERT(&vm == this->vm());
- ASSERT(const_cast<MarkedBlock*>(this)->handle().cellAlign(cell) == cell);
+ RELEASE_ASSERT(&vm == this->vm());
+ RELEASE_ASSERT(const_cast<MarkedBlock*>(this)->handle().cellAlign(cell) == cell);
}
#endif
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
