<!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>[207408] 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/207408">207408</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-10-17 09:19:10 -0700 (Mon, 17 Oct 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Air::IRC needs to place all Tmps on some worklist, even if they have no interference edges
https://bugs.webkit.org/show_bug.cgi?id=163509
Reviewed by Mark Lam.
The worklist building function in IRC skips temporaries that have no degree. This doesn't appear
to be necessary. This has been there since the original IRC commit. It hasn't caused bugs because
ordinarily, the only way to have a tmp with no degree is to not have any mention of that tmp. But
while working on bug 163371, I hit a crazy corner case where a temporary would have no
interference edges (i.e. no degree). Here's how it happens:
A spill tmp from a previous iteration of IRC may have no degree: imagine a tmp that is live
everywhere and interferes with everyone, but has one use like:
Move %ourTmp, %someOtherTmp
Where there are no other tmps live. After spill conversion, this may look like:
Move (ourSpill), %newTmp
Move %newTmp, %someOtherTmp
Of course, we'd rather not get this kind of spill code but it's totally possible because we now
have a bunch of random conditions under which we won't slap the spill address directly into the
Move.
After this happens, assuming that the only thing live was %someOtherTmp, we will have zero degree
for %newTmp because the Move is coalescable and does not contribute to interference.
Then, we might coalesce %someOtherTmp with %newTmp. Once this happens, if we make the %newTmp be
the master, we're in deep trouble because %newTmp is not on any worklist.
I don't know how to reproduce this except through the patch in bug 163371. Removing the two lines
of code that skipped no-degree tmps causes no regressions, and resolves the problem I was having.
* b3/air/AirIteratedRegisterCoalescing.cpp:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirIteratedRegisterCoalescingcpp">trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (207407 => 207408)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-10-17 11:59:58 UTC (rev 207407)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-10-17 16:19:10 UTC (rev 207408)
</span><span class="lines">@@ -1,3 +1,41 @@
</span><ins>+2016-10-16 Filip Pizlo <fpizlo@apple.com>
+
+ Air::IRC needs to place all Tmps on some worklist, even if they have no interference edges
+ https://bugs.webkit.org/show_bug.cgi?id=163509
+
+ Reviewed by Mark Lam.
+
+ The worklist building function in IRC skips temporaries that have no degree. This doesn't appear
+ to be necessary. This has been there since the original IRC commit. It hasn't caused bugs because
+ ordinarily, the only way to have a tmp with no degree is to not have any mention of that tmp. But
+ while working on bug 163371, I hit a crazy corner case where a temporary would have no
+ interference edges (i.e. no degree). Here's how it happens:
+
+ A spill tmp from a previous iteration of IRC may have no degree: imagine a tmp that is live
+ everywhere and interferes with everyone, but has one use like:
+
+ Move %ourTmp, %someOtherTmp
+
+ Where there are no other tmps live. After spill conversion, this may look like:
+
+ Move (ourSpill), %newTmp
+ Move %newTmp, %someOtherTmp
+
+ Of course, we'd rather not get this kind of spill code but it's totally possible because we now
+ have a bunch of random conditions under which we won't slap the spill address directly into the
+ Move.
+
+ After this happens, assuming that the only thing live was %someOtherTmp, we will have zero degree
+ for %newTmp because the Move is coalescable and does not contribute to interference.
+
+ Then, we might coalesce %someOtherTmp with %newTmp. Once this happens, if we make the %newTmp be
+ the master, we're in deep trouble because %newTmp is not on any worklist.
+
+ I don't know how to reproduce this except through the patch in bug 163371. Removing the two lines
+ of code that skipped no-degree tmps causes no regressions, and resolves the problem I was having.
+
+ * b3/air/AirIteratedRegisterCoalescing.cpp:
+
</ins><span class="cx"> 2016-10-15 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> Add a $vm.getpid() method.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirIteratedRegisterCoalescingcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp (207407 => 207408)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp 2016-10-17 11:59:58 UTC (rev 207407)
+++ trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp 2016-10-17 16:19:10 UTC (rev 207408)
</span><span class="lines">@@ -88,9 +88,6 @@
</span><span class="cx"> IndexType firstNonRegIndex = m_lastPrecoloredRegisterIndex + 1;
</span><span class="cx"> for (IndexType i = firstNonRegIndex; i < m_degrees.size(); ++i) {
</span><span class="cx"> unsigned degree = m_degrees[i];
</span><del>- if (!degree)
- continue;
-
</del><span class="cx"> if (degree >= m_regsInPriorityOrder.size())
</span><span class="cx"> addToSpill(i);
</span><span class="cx"> else if (!m_moveList[i].isEmpty())
</span></span></pre>
</div>
</div>
</body>
</html>