<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[209595] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/209595">209595</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-12-08 20:53:33 -0800 (Thu, 08 Dec 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGFixupPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGStoreBarrierInsertionPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMarkedBlockcpp">trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (209594 => 209595)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-12-09 04:53:33 UTC (rev 209595)
</span><span class="lines">@@ -1,5 +1,33 @@
</span><span class="cx"> 2016-12-08 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><ins>+ 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 <fpizlo@apple.com>
+
</ins><span class="cx"> Enable concurrent GC on ARM64
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=165643
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGFixupPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (209594 => 209595)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp 2016-12-09 04:53:33 UTC (rev 209595)
</span><span class="lines">@@ -1335,7 +1335,6 @@
</span><span class="cx">
</span><span class="cx"> case MultiPutByOffset: {
</span><span class="cx"> fixEdge<CellUse>(node->child1());
</span><del>- speculateForBarrier(node->child2());
</del><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGStoreBarrierInsertionPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cpp (209594 => 209595)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cpp 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cpp 2016-12-09 04:53:33 UTC (rev 209595)
</span><span class="lines">@@ -272,12 +272,16 @@
</span><span class="cx">
</span><span class="cx"> case PutClosureVar:
</span><span class="cx"> case PutToArguments:
</span><del>- case SetRegExpObjectLastIndex:
- case MultiPutByOffset: {
</del><ins>+ case SetRegExpObjectLastIndex: {
</ins><span class="cx"> considerBarrier(m_node->child1(), m_node->child2());
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ case MultiPutByOffset: {
+ considerBarrier(m_node->child1());
+ break;
+ }
+
</ins><span class="cx"> case PutByOffset: {
</span><span class="cx"> considerBarrier(m_node->child2(), m_node->child3());
</span><span class="cx"> break;
</span><span class="lines">@@ -325,7 +329,6 @@
</span><span class="cx">
</span><span class="cx"> case AllocatePropertyStorage:
</span><span class="cx"> case ReallocatePropertyStorage:
</span><del>- // These allocate but then run their own barrier.
</del><span class="cx"> insertBarrier(m_nodeIndex + 1, m_node->child1());
</span><span class="cx"> m_node->setEpoch(Epoch());
</span><span class="cx"> break;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMarkedBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp (209594 => 209595)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp 2016-12-09 03:30:03 UTC (rev 209594)
+++ trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp 2016-12-09 04:53:33 UTC (rev 209595)
</span><span class="lines">@@ -568,8 +568,8 @@
</span><span class="cx"> #if !ASSERT_DISABLED
</span><span class="cx"> void MarkedBlock::assertValidCell(VM& vm, HeapCell* cell) const
</span><span class="cx"> {
</span><del>- ASSERT(&vm == this->vm());
- ASSERT(const_cast<MarkedBlock*>(this)->handle().cellAlign(cell) == cell);
</del><ins>+ RELEASE_ASSERT(&vm == this->vm());
+ RELEASE_ASSERT(const_cast<MarkedBlock*>(this)->handle().cellAlign(cell) == cell);
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>