Title: [282941] branches/safari-612-branch
- Revision
- 282941
- Author
- repst...@apple.com
- Date
- 2021-09-22 22:14:20 -0700 (Wed, 22 Sep 2021)
Log Message
Cherry-pick r282663. rdar://problem/83429716
PutByVal and PutPrivateName ICs should emit a write barrier if a butterfly might be allocated
https://bugs.webkit.org/show_bug.cgi?id=230378
Reviewed by Yusuke Suzuki.
Right now, PutByVal and PutPrivateName check the value type to determine
if a write barrier is needed. For example, putting a primitive is considered
to not require a write barrier. This makes sense, except for the case when we
might allocate or re-allocate a butterfly in the IC. This does not emit a write
barrier, and so the GC might miss the new butterfly. That is somewhat undesirable.
This is a temporary conservative fix. If we don't write to the butterfly pointer,
then we still don't need a write barrier; this work is captured by
https://bugs.webkit.org/show_bug.cgi?id=230377
* dfg/DFGStoreBarrierInsertionPhase.cpp:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282663 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Added: branches/safari-612-branch/JSTests/stress/put-by-val-ic-write-barrier.js (0 => 282941)
--- branches/safari-612-branch/JSTests/stress/put-by-val-ic-write-barrier.js (rev 0)
+++ branches/safari-612-branch/JSTests/stress/put-by-val-ic-write-barrier.js 2021-09-23 05:14:20 UTC (rev 282941)
@@ -0,0 +1,22 @@
+//@ runDefault("--collectContinuously=1", "--useAccessInlining=0", "--verifyGC=1")
+
+function PutByValICPrimitive() {
+ let leak = []
+
+ function doByVal(o, s) {
+ o[s] = 1337
+ }
+ noInline(doByVal)
+
+ for (let i = 0; i < 1000000; ++i) {
+ let o1 = {a: 1, b: 2}
+ let o2 = {a: 1, b: 2}
+ let o3 = {a: 1, b: 2}
+ doByVal(o1, "x")
+ doByVal(o2, "y")
+ doByVal(o3, "z")
+ leak.push(o1, o2, o3)
+ }
+}
+noInline(PutByValICPrimitive)
+PutByValICPrimitive()
\ No newline at end of file
Added: branches/safari-612-branch/JSTests/stress/put-private-name-ic-write-barrier.js (0 => 282941)
--- branches/safari-612-branch/JSTests/stress/put-private-name-ic-write-barrier.js (rev 0)
+++ branches/safari-612-branch/JSTests/stress/put-private-name-ic-write-barrier.js 2021-09-23 05:14:20 UTC (rev 282941)
@@ -0,0 +1,29 @@
+//@ runDefault("--collectContinuously=1", "--usePolyvariantDevirtualization=0", "--forceDebuggerBytecodeGeneration=1", "--verifyGC=1")
+// UsePolyvariantDevirtualization gives us a PutPrivateName (not byID) while still letting us generate an IC with only one AccessCase
+// DebuggerBytecodeGeneration seems to give the GC more time to interrupt the put because it forces reads from the stack
+
+function PutPrivateNameIC() {
+ let leak = []
+
+ class A {
+ constructor() {
+ this.a = 0
+ }
+ }
+ noInline(A)
+
+ class B extends A {
+ #b
+ #c
+ }
+ noInline(B)
+
+ for (let i = 0; i < 1000000; ++i) {
+ let b1 = new B
+ let b2 = new B
+ let b3 = new B
+ leak.push(b1, b2, b3)
+ }
+}
+noInline(PutPrivateNameIC)
+PutPrivateNameIC()
\ No newline at end of file
Modified: branches/safari-612-branch/Source/_javascript_Core/ChangeLog (282940 => 282941)
--- branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2021-09-23 05:14:17 UTC (rev 282940)
+++ branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2021-09-23 05:14:20 UTC (rev 282941)
@@ -1,5 +1,46 @@
2021-09-22 Alan Coon <alanc...@apple.com>
+ Cherry-pick r282663. rdar://problem/83429716
+
+ PutByVal and PutPrivateName ICs should emit a write barrier if a butterfly might be allocated
+ https://bugs.webkit.org/show_bug.cgi?id=230378
+
+ Reviewed by Yusuke Suzuki.
+
+ Right now, PutByVal and PutPrivateName check the value type to determine
+ if a write barrier is needed. For example, putting a primitive is considered
+ to not require a write barrier. This makes sense, except for the case when we
+ might allocate or re-allocate a butterfly in the IC. This does not emit a write
+ barrier, and so the GC might miss the new butterfly. That is somewhat undesirable.
+ This is a temporary conservative fix. If we don't write to the butterfly pointer,
+ then we still don't need a write barrier; this work is captured by
+ https://bugs.webkit.org/show_bug.cgi?id=230377
+
+ * dfg/DFGStoreBarrierInsertionPhase.cpp:
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282663 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-09-17 Justin Michaud <justin_mich...@apple.com>
+
+ PutByVal and PutPrivateName ICs should emit a write barrier if a butterfly might be allocated
+ https://bugs.webkit.org/show_bug.cgi?id=230378
+
+ Reviewed by Yusuke Suzuki.
+
+ Right now, PutByVal and PutPrivateName check the value type to determine
+ if a write barrier is needed. For example, putting a primitive is considered
+ to not require a write barrier. This makes sense, except for the case when we
+ might allocate or re-allocate a butterfly in the IC. This does not emit a write
+ barrier, and so the GC might miss the new butterfly. That is somewhat undesirable.
+ This is a temporary conservative fix. If we don't write to the butterfly pointer,
+ then we still don't need a write barrier; this work is captured by
+ https://bugs.webkit.org/show_bug.cgi?id=230377
+
+ * dfg/DFGStoreBarrierInsertionPhase.cpp:
+
+2021-09-22 Alan Coon <alanc...@apple.com>
+
Cherry-pick r282621. rdar://problem/83183656
Move some profiling to UnlinkedCodeBlock
Modified: branches/safari-612-branch/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp (282940 => 282941)
--- branches/safari-612-branch/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp 2021-09-23 05:14:17 UTC (rev 282940)
+++ branches/safari-612-branch/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp 2021-09-23 05:14:20 UTC (rev 282941)
@@ -237,9 +237,9 @@
case Array::BigInt64Array:
case Array::BigUint64Array: {
Edge child1 = m_graph.varArgChild(m_node, 0);
- Edge child3 = m_graph.varArgChild(m_node, 2);
if (!m_graph.m_slowPutByVal.contains(m_node) && (child1.useKind() == CellUse || child1.useKind() == KnownCellUse))
- considerBarrier(child1, child3);
+ // FIXME: there are some cases where we can avoid a store barrier by considering the value https://bugs.webkit.org/show_bug.cgi?id=230377
+ considerBarrier(child1);
break;
}
case Array::Contiguous:
@@ -277,7 +277,8 @@
case PutPrivateName: {
if (!m_graph.m_slowPutByVal.contains(m_node) && (m_node->child1().useKind() == CellUse || m_node->child1().useKind() == KnownCellUse))
- considerBarrier(m_node->child1(), m_node->child3());
+ // FIXME: there are some cases where we can avoid a store barrier by considering the value https://bugs.webkit.org/show_bug.cgi?id=230377
+ considerBarrier(m_node->child1());
break;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
