<!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>[192497] 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/192497">192497</a></dd>
<dt>Author</dt> <dd>benjamin@webkit.org</dd>
<dt>Date</dt> <dd>2015-11-16 16:44:28 -0800 (Mon, 16 Nov 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>[JSC] Add support for the extra registers that can be clobbered by Specials
https://bugs.webkit.org/show_bug.cgi?id=151246

Patch by Benjamin Poulain &lt;bpoulain@apple.com&gt; on 2015-11-16
Reviewed by Geoffrey Garen.

Specials can clobber arbitrary registers. This was not handled correctly by Air
and nothing was preventing us from re-allocating those registers.

This patch adds support for the extra clobbered registers in the two register allocators.

I also fixed the re-spilling FIXME of the iterated allocator because the test might
not always converge without it. Since we are at maximum register pressure at the patch point,
we could be always spilling the return value, which would loop forever.

To fix the re-spilling, I just kept a Set of every value spilled or filled so far. When selecting
a spill candidate, we never pick a Tmp from that set.

* b3/air/AirGenerate.cpp:
(JSC::B3::Air::generate):
* b3/air/AirHandleCalleeSaves.cpp:
(JSC::B3::Air::handleCalleeSaves):
* b3/air/AirInst.h:
* b3/air/AirInstInlines.h:
(JSC::B3::Air::Inst::forEachDefAndExtraClobberedTmp):
* b3/air/AirIteratedRegisterCoalescing.cpp:
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::IteratedRegisterCoalescingAllocator):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::build):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::addEdges):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::selectSpill):
(JSC::B3::Air::addSpillAndFillToProgram):
(JSC::B3::Air::iteratedRegisterCoalescingOnType):
(JSC::B3::Air::iteratedRegisterCoalescing):
* b3/air/AirSpillEverything.cpp:
(JSC::B3::Air::spillEverything):
* b3/testb3.cpp:
(JSC::B3::testSimplePatchpointWithoutOuputClobbersGPArgs):
(JSC::B3::testSimplePatchpointWithOuputClobbersGPArgs):
(JSC::B3::testSimplePatchpointWithoutOuputClobbersFPArgs):
(JSC::B3::testSimplePatchpointWithOuputClobbersFPArgs):
(JSC::B3::run):
* jit/RegisterSet.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirGeneratecpp">trunk/Source/JavaScriptCore/b3/air/AirGenerate.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirHandleCalleeSavescpp">trunk/Source/JavaScriptCore/b3/air/AirHandleCalleeSaves.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirInsth">trunk/Source/JavaScriptCore/b3/air/AirInst.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirInstInlinesh">trunk/Source/JavaScriptCore/b3/air/AirInstInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirIteratedRegisterCoalescingcpp">trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirSpillEverythingcpp">trunk/Source/JavaScriptCore/b3/air/AirSpillEverything.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3testb3cpp">trunk/Source/JavaScriptCore/b3/testb3.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRegisterSeth">trunk/Source/JavaScriptCore/jit/RegisterSet.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/ChangeLog        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -1,3 +1,47 @@
</span><ins>+2015-11-16  Benjamin Poulain  &lt;bpoulain@apple.com&gt;
+
+        [JSC] Add support for the extra registers that can be clobbered by Specials
+        https://bugs.webkit.org/show_bug.cgi?id=151246
+
+        Reviewed by Geoffrey Garen.
+
+        Specials can clobber arbitrary registers. This was not handled correctly by Air
+        and nothing was preventing us from re-allocating those registers.
+
+        This patch adds support for the extra clobbered registers in the two register allocators.
+
+        I also fixed the re-spilling FIXME of the iterated allocator because the test might
+        not always converge without it. Since we are at maximum register pressure at the patch point,
+        we could be always spilling the return value, which would loop forever.
+
+        To fix the re-spilling, I just kept a Set of every value spilled or filled so far. When selecting
+        a spill candidate, we never pick a Tmp from that set.
+
+        * b3/air/AirGenerate.cpp:
+        (JSC::B3::Air::generate):
+        * b3/air/AirHandleCalleeSaves.cpp:
+        (JSC::B3::Air::handleCalleeSaves):
+        * b3/air/AirInst.h:
+        * b3/air/AirInstInlines.h:
+        (JSC::B3::Air::Inst::forEachDefAndExtraClobberedTmp):
+        * b3/air/AirIteratedRegisterCoalescing.cpp:
+        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::IteratedRegisterCoalescingAllocator):
+        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::build):
+        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::addEdges):
+        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::selectSpill):
+        (JSC::B3::Air::addSpillAndFillToProgram):
+        (JSC::B3::Air::iteratedRegisterCoalescingOnType):
+        (JSC::B3::Air::iteratedRegisterCoalescing):
+        * b3/air/AirSpillEverything.cpp:
+        (JSC::B3::Air::spillEverything):
+        * b3/testb3.cpp:
+        (JSC::B3::testSimplePatchpointWithoutOuputClobbersGPArgs):
+        (JSC::B3::testSimplePatchpointWithOuputClobbersGPArgs):
+        (JSC::B3::testSimplePatchpointWithoutOuputClobbersFPArgs):
+        (JSC::B3::testSimplePatchpointWithOuputClobbersFPArgs):
+        (JSC::B3::run):
+        * jit/RegisterSet.h:
+
</ins><span class="cx"> 2015-11-16  Benjamin Poulain  &lt;benjamin@webkit.org&gt;
</span><span class="cx"> 
</span><span class="cx">         Build fix after r192492
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirGeneratecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirGenerate.cpp (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirGenerate.cpp        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/b3/air/AirGenerate.cpp        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -67,8 +67,10 @@
</span><span class="cx">     
</span><span class="cx">     eliminateDeadCode(code);
</span><span class="cx"> 
</span><del>-    // This is where we would have a real register allocator. Then, we could use spillEverything()
-    // in place of the register allocator only for testing.
</del><ins>+    // Register allocation for all the Tmps that do not have a corresponding machine register.
+    // After this phase, every Tmp has a reg.
+    //
+    // For debugging, you can use spillEverything() to put everything to the stack between each Inst.
</ins><span class="cx">     iteratedRegisterCoalescing(code);
</span><span class="cx"> 
</span><span class="cx">     // Prior to this point the prologue and epilogue is implicit. This makes it explicit. It also
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirHandleCalleeSavescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirHandleCalleeSaves.cpp (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirHandleCalleeSaves.cpp        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/b3/air/AirHandleCalleeSaves.cpp        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -30,6 +30,7 @@
</span><span class="cx"> 
</span><span class="cx"> #include &quot;AirCode.h&quot;
</span><span class="cx"> #include &quot;AirInsertionSet.h&quot;
</span><ins>+#include &quot;AirInstInlines.h&quot;
</ins><span class="cx"> #include &quot;AirPhaseScope.h&quot;
</span><span class="cx"> 
</span><span class="cx"> namespace JSC { namespace B3 { namespace Air {
</span><span class="lines">@@ -47,6 +48,9 @@
</span><span class="cx">                     // At first we just record all used regs.
</span><span class="cx">                     usedCalleeSaves.set(tmp.reg());
</span><span class="cx">                 });
</span><ins>+
+            if (inst.hasSpecial())
+                usedCalleeSaves.merge(inst.extraClobberedRegs());
</ins><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirInsth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirInst.h (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirInst.h        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/b3/air/AirInst.h        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -126,6 +126,10 @@
</span><span class="cx">     // extraClobberedRegs() only works if hasSpecial() returns true.
</span><span class="cx">     const RegisterSet&amp; extraClobberedRegs();
</span><span class="cx"> 
</span><ins>+    // Iterate over the Defs and the extra clobbered registers.
+    template&lt;typename Functor&gt;
+    void forEachDefAndExtraClobberedTmp(Arg::Type, const Functor&amp; functor);
+
</ins><span class="cx">     // Use this to report which registers are live. This should be done just before codegen. Note
</span><span class="cx">     // that for efficiency, reportUsedRegisters() only works if hasSpecial() returns true.
</span><span class="cx">     void reportUsedRegisters(const RegisterSet&amp;);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirInstInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirInstInlines.h (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirInstInlines.h        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/b3/air/AirInstInlines.h        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -89,6 +89,26 @@
</span><span class="cx">     return args[0].special()-&gt;extraClobberedRegs(*this);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+template&lt;typename Functor&gt;
+inline void Inst::forEachDefAndExtraClobberedTmp(Arg::Type type, const Functor&amp; functor)
+{
+    forEachTmp([&amp;] (Tmp&amp; tmpArg, Arg::Role role, Arg::Type argType) {
+        if (argType == type &amp;&amp; Arg::isDef(role))
+            functor(tmpArg);
+    });
+
+    if (!hasSpecial())
+        return;
+
+    const RegisterSet&amp; clobberedRegisters = extraClobberedRegs();
+    clobberedRegisters.forEach([functor, type] (Reg reg) {
+        if (reg.isGPR() == (type == Arg::GP)) {
+            Tmp registerTmp(reg);
+            functor(registerTmp);
+        }
+    });
+}
+
</ins><span class="cx"> inline void Inst::reportUsedRegisters(const RegisterSet&amp; usedRegisters)
</span><span class="cx"> {
</span><span class="cx">     ASSERT(hasSpecial());
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirIteratedRegisterCoalescingcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -130,8 +130,9 @@
</span><span class="cx"> template&lt;Arg::Type type&gt;
</span><span class="cx"> class IteratedRegisterCoalescingAllocator {
</span><span class="cx"> public:
</span><del>-    IteratedRegisterCoalescingAllocator(Code&amp; code)
-        : m_numberOfRegisters(regsInPriorityOrder(type).size())
</del><ins>+    IteratedRegisterCoalescingAllocator(Code&amp; code, const HashSet&lt;Tmp&gt;&amp; unspillableTmp)
+        : m_unspillableTmp(unspillableTmp)
+        , m_numberOfRegisters(regsInPriorityOrder(type).size())
</ins><span class="cx">     {
</span><span class="cx">         initializeDegrees(code);
</span><span class="cx"> 
</span><span class="lines">@@ -144,20 +145,17 @@
</span><span class="cx"> 
</span><span class="cx">     void build(Inst&amp; inst, const Liveness&lt;Tmp&gt;::LocalCalc&amp; localCalc)
</span><span class="cx">     {
</span><del>-        // All the Def()s interfere with eachother.
-        inst.forEachTmp([&amp;] (Tmp&amp; arg, Arg::Role role, Arg::Type argType) {
-            if (argType != type)
-                return;
</del><ins>+        inst.forEachDefAndExtraClobberedTmp(type, [&amp;] (Tmp&amp; arg) {
+            // All the Def()s interfere with eachother and with all the extra clobbered Tmps.
+            // We should not use forEachDefAndExtraClobberedTmp() here since colored Tmps
+            // do not need interference edges in our implementation.
+            inst.forEachTmp([&amp;] (Tmp&amp; otherArg, Arg::Role role, Arg::Type otherArgType) {
+                if (otherArgType != type)
+                    return;
</ins><span class="cx"> 
</span><del>-            if (Arg::isDef(role)) {
-                inst.forEachTmp([&amp;] (Tmp&amp; otherArg, Arg::Role role, Arg::Type) {
-                    if (argType != type)
-                        return;
-
-                    if (Arg::isDef(role))
-                        addEdge(arg, otherArg);
-                });
-            }
</del><ins>+                if (Arg::isDef(role))
+                    addEdge(arg, otherArg);
+            });
</ins><span class="cx">         });
</span><span class="cx"> 
</span><span class="cx">         if (MoveInstHelper&lt;type&gt;::mayBeCoalescable(inst)) {
</span><span class="lines">@@ -282,12 +280,10 @@
</span><span class="cx">     void addEdges(Inst&amp; inst, const HashSet&lt;Tmp&gt;&amp; liveTmp)
</span><span class="cx">     {
</span><span class="cx">         // All the Def()s interfere with everthing live.
</span><del>-        inst.forEachTmp([&amp;] (Tmp&amp; arg, Arg::Role role, Arg::Type argType) {
-            if (argType == type &amp;&amp; Arg::isDef(role)) {
-                for (const Tmp&amp; liveTmp : liveTmp) {
-                    if (liveTmp.isGP() == (type == Arg::GP))
-                        addEdge(arg, liveTmp);
-                }
</del><ins>+        inst.forEachDefAndExtraClobberedTmp(type, [&amp;] (Tmp&amp; arg) {
+            for (const Tmp&amp; liveTmp : liveTmp) {
+                if (liveTmp.isGP() == (type == Arg::GP))
+                    addEdge(arg, liveTmp);
</ins><span class="cx">             }
</span><span class="cx">         });
</span><span class="cx">     }
</span><span class="lines">@@ -573,10 +569,13 @@
</span><span class="cx">     void selectSpill()
</span><span class="cx">     {
</span><span class="cx">         // FIXME: we should select a good candidate based on all the information we have.
</span><del>-        // FIXME: we should never select a spilled tmp as we would never converge.
-
</del><span class="cx">         auto iterator = m_spillWorklist.begin();
</span><span class="cx"> 
</span><ins>+        while (iterator != m_spillWorklist.end() &amp;&amp; m_unspillableTmp.contains(*iterator))
+            ++iterator;
+
+        RELEASE_ASSERT_WITH_MESSAGE(iterator != m_spillWorklist.end(), &quot;It is not possible to color the Air graph with the number of available registers.&quot;);
+
</ins><span class="cx">         auto victimIterator = iterator;
</span><span class="cx">         unsigned maxDegree = m_degrees[AbsoluteTmpHelper&lt;type&gt;::absoluteIndex(*iterator)];
</span><span class="cx"> 
</span><span class="lines">@@ -584,6 +583,9 @@
</span><span class="cx">         for (;iterator != m_spillWorklist.end(); ++iterator) {
</span><span class="cx">             unsigned tmpDegree = m_degrees[AbsoluteTmpHelper&lt;type&gt;::absoluteIndex(*iterator)];
</span><span class="cx">             if (tmpDegree &gt; maxDegree) {
</span><ins>+                if (m_unspillableTmp.contains(*iterator))
+                    continue;
+
</ins><span class="cx">                 victimIterator = iterator;
</span><span class="cx">                 maxDegree = tmpDegree;
</span><span class="cx">             }
</span><span class="lines">@@ -749,6 +751,7 @@
</span><span class="cx">     };
</span><span class="cx">     typedef SimpleClassHashTraits&lt;InterferenceEdge&gt; InterferenceEdgeHashTraits;
</span><span class="cx"> 
</span><ins>+    const HashSet&lt;Tmp&gt;&amp; m_unspillableTmp;
</ins><span class="cx">     unsigned m_numberOfRegisters { 0 };
</span><span class="cx"> 
</span><span class="cx">     // The interference graph.
</span><span class="lines">@@ -901,8 +904,11 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template&lt;Arg::Type type&gt;
</span><del>-static void addSpillAndFillToProgram(Code&amp; code, const HashSet&lt;Tmp&gt;&amp; spilledTmp)
</del><ins>+static void addSpillAndFillToProgram(Code&amp; code, const HashSet&lt;Tmp&gt;&amp; spilledTmp, HashSet&lt;Tmp&gt;&amp; unspillableTmp)
</ins><span class="cx"> {
</span><ins>+    // All the spilled values become unspillable.
+    unspillableTmp.add(spilledTmp.begin(), spilledTmp.end());
+
</ins><span class="cx">     // Allocate stack slot for each spilled value.
</span><span class="cx">     HashMap&lt;Tmp, StackSlot*&gt; stackSlots;
</span><span class="cx">     for (Tmp tmp : spilledTmp) {
</span><span class="lines">@@ -947,6 +953,9 @@
</span><span class="cx">                     Tmp newTmp = code.newTmp(type);
</span><span class="cx">                     insertionSet.insert(instIndex, move, inst.origin, arg, newTmp);
</span><span class="cx">                     tmp = newTmp;
</span><ins>+
+                    // Any new Fill() should never be spilled.
+                    unspillableTmp.add(tmp);
</ins><span class="cx">                 }
</span><span class="cx">                 if (Arg::isDef(role))
</span><span class="cx">                     insertionSet.insert(instIndex + 1, move, inst.origin, tmp, arg);
</span><span class="lines">@@ -957,10 +966,10 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template&lt;Arg::Type type&gt;
</span><del>-static void iteratedRegisterCoalescingOnType(Code&amp; code)
</del><ins>+static void iteratedRegisterCoalescingOnType(Code&amp; code, HashSet&lt;Tmp&gt;&amp; unspillableTmps)
</ins><span class="cx"> {
</span><span class="cx">     while (true) {
</span><del>-        IteratedRegisterCoalescingAllocator&lt;type&gt; allocator(code);
</del><ins>+        IteratedRegisterCoalescingAllocator&lt;type&gt; allocator(code, unspillableTmps);
</ins><span class="cx">         Liveness&lt;Tmp&gt; liveness(code);
</span><span class="cx">         for (BasicBlock* block : code) {
</span><span class="cx">             Liveness&lt;Tmp&gt;::LocalCalc localCalc(liveness, block);
</span><span class="lines">@@ -976,7 +985,7 @@
</span><span class="cx">             assignRegisterToTmpInProgram(code, allocator);
</span><span class="cx">             return;
</span><span class="cx">         }
</span><del>-        addSpillAndFillToProgram&lt;type&gt;(code, allocator.spilledTmp());
</del><ins>+        addSpillAndFillToProgram&lt;type&gt;(code, allocator.spilledTmp(), unspillableTmps);
</ins><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -987,10 +996,13 @@
</span><span class="cx">     bool gpIsColored = false;
</span><span class="cx">     bool fpIsColored = false;
</span><span class="cx"> 
</span><ins>+    HashSet&lt;Tmp&gt; unspillableGPs;
+    HashSet&lt;Tmp&gt; unspillableFPs;
+
</ins><span class="cx">     // First we run both allocator together as long as they both spill.
</span><span class="cx">     while (!gpIsColored &amp;&amp; !fpIsColored) {
</span><del>-        IteratedRegisterCoalescingAllocator&lt;Arg::GP&gt; gpAllocator(code);
-        IteratedRegisterCoalescingAllocator&lt;Arg::FP&gt; fpAllocator(code);
</del><ins>+        IteratedRegisterCoalescingAllocator&lt;Arg::GP&gt; gpAllocator(code, unspillableGPs);
+        IteratedRegisterCoalescingAllocator&lt;Arg::FP&gt; fpAllocator(code, unspillableFPs);
</ins><span class="cx"> 
</span><span class="cx">         // Liveness Analysis can be prohibitively expensive. It is shared
</span><span class="cx">         // between the two allocators to avoid doing it twice.
</span><span class="lines">@@ -1014,19 +1026,19 @@
</span><span class="cx">             assignRegisterToTmpInProgram(code, gpAllocator);
</span><span class="cx">             gpIsColored = true;
</span><span class="cx">         } else
</span><del>-            addSpillAndFillToProgram&lt;Arg::GP&gt;(code, gpAllocator.spilledTmp());
</del><ins>+            addSpillAndFillToProgram&lt;Arg::GP&gt;(code, gpAllocator.spilledTmp(), unspillableGPs);
</ins><span class="cx"> 
</span><span class="cx">         if (fpAllocator.spilledTmp().isEmpty()) {
</span><span class="cx">             assignRegisterToTmpInProgram(code, fpAllocator);
</span><span class="cx">             fpIsColored = true;
</span><span class="cx">         } else
</span><del>-            addSpillAndFillToProgram&lt;Arg::FP&gt;(code, fpAllocator.spilledTmp());
</del><ins>+            addSpillAndFillToProgram&lt;Arg::FP&gt;(code, fpAllocator.spilledTmp(), unspillableFPs);
</ins><span class="cx">     };
</span><span class="cx"> 
</span><span class="cx">     if (!gpIsColored)
</span><del>-        iteratedRegisterCoalescingOnType&lt;Arg::GP&gt;(code);
</del><ins>+        iteratedRegisterCoalescingOnType&lt;Arg::GP&gt;(code, unspillableGPs);
</ins><span class="cx">     if (!fpIsColored)
</span><del>-        iteratedRegisterCoalescingOnType&lt;Arg::FP&gt;(code);
</del><ins>+        iteratedRegisterCoalescingOnType&lt;Arg::FP&gt;(code, unspillableFPs);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> } } } // namespace JSC::B3::Air
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirSpillEverythingcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirSpillEverything.cpp (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirSpillEverything.cpp        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/b3/air/AirSpillEverything.cpp        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -58,11 +58,12 @@
</span><span class="cx"> 
</span><span class="cx">             // Gotta account for dead assignments to registers. These may happen because the input
</span><span class="cx">             // code is suboptimal.
</span><del>-            inst.forEachArg(
-                [&amp;] (Arg&amp; arg, Arg::Role role, Arg::Type) {
-                    if (Arg::isDef(role) &amp;&amp; arg.isReg())
-                        registerSet.set(arg.reg());
-                });
</del><ins>+            auto updateRegisterSet = [&amp;registerSet] (const Tmp&amp; tmp) {
+                if (tmp.isReg())
+                    registerSet.set(tmp.reg());
+            };
+            inst.forEachDefAndExtraClobberedTmp(Arg::GP, updateRegisterSet);
+            inst.forEachDefAndExtraClobberedTmp(Arg::FP, updateRegisterSet);
</ins><span class="cx">         };
</span><span class="cx"> 
</span><span class="cx">         for (unsigned instIndex = block-&gt;size(); instIndex--;) {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3testb3cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/testb3.cpp (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/testb3.cpp        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/b3/testb3.cpp        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -2760,6 +2760,154 @@
</span><span class="cx">     CHECK(compileAndRun&lt;int&gt;(proc, 1, 2) == 3);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+void testSimplePatchpointWithoutOuputClobbersGPArgs()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), GPRInfo::argumentGPR0);
+    Value* arg2 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), GPRInfo::argumentGPR1);
+    Value* const1 = root-&gt;appendNew&lt;Const64Value&gt;(proc, Origin(), 42);
+    Value* const2 = root-&gt;appendNew&lt;Const64Value&gt;(proc, Origin(), 13);
+
+    PatchpointValue* patchpoint = root-&gt;appendNew&lt;PatchpointValue&gt;(proc, Void, Origin());
+    patchpoint-&gt;clobber(RegisterSet(GPRInfo::argumentGPR0, GPRInfo::argumentGPR1));
+    patchpoint-&gt;append(ConstrainedValue(const1, ValueRep::SomeRegister));
+    patchpoint-&gt;append(ConstrainedValue(const2, ValueRep::SomeRegister));
+    patchpoint-&gt;setGenerator(
+        [&amp;] (CCallHelpers&amp; jit, const StackmapGenerationParams&amp; params) {
+            CHECK(params.reps.size() == 2);
+            CHECK(params.reps[0].isGPR());
+            CHECK(params.reps[1].isGPR());
+            jit.move(CCallHelpers::TrustedImm32(0x00ff00ff), params.reps[0].gpr());
+            jit.move(CCallHelpers::TrustedImm32(0x00ff00ff), params.reps[1].gpr());
+            jit.move(CCallHelpers::TrustedImm32(0x00ff00ff), GPRInfo::argumentGPR0);
+            jit.move(CCallHelpers::TrustedImm32(0x00ff00ff), GPRInfo::argumentGPR1);
+        });
+
+    Value* result = root-&gt;appendNew&lt;Value&gt;(proc, Add, Origin(), arg1, arg2);
+    root-&gt;appendNew&lt;ControlValue&gt;(proc, Return, Origin(), result);
+
+    CHECK(compileAndRun&lt;int&gt;(proc, 1, 2) == 3);
+}
+
+void testSimplePatchpointWithOuputClobbersGPArgs()
+{
+    // We can't predict where the output will be but we want to be sure it is not
+    // one of the clobbered registers which is a bit hard to test.
+    //
+    // What we do is force the hand of our register allocator by clobbering absolutely
+    // everything but 1. The only valid allocation is to give it to the result and
+    // spill everything else.
+
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), GPRInfo::argumentGPR0);
+    Value* arg2 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), GPRInfo::argumentGPR1);
+    Value* const1 = root-&gt;appendNew&lt;Const64Value&gt;(proc, Origin(), 42);
+    Value* const2 = root-&gt;appendNew&lt;Const64Value&gt;(proc, Origin(), 13);
+
+    PatchpointValue* patchpoint = root-&gt;appendNew&lt;PatchpointValue&gt;(proc, Int64, Origin());
+
+    RegisterSet clobberAll = RegisterSet::allGPRs();
+    clobberAll.exclude(RegisterSet::stackRegisters());
+    clobberAll.exclude(RegisterSet::reservedHardwareRegisters());
+    clobberAll.clear(GPRInfo::argumentGPR2);
+    patchpoint-&gt;clobber(clobberAll);
+
+    patchpoint-&gt;append(ConstrainedValue(const1, ValueRep::SomeRegister));
+    patchpoint-&gt;append(ConstrainedValue(const2, ValueRep::SomeRegister));
+
+    patchpoint-&gt;setGenerator(
+        [&amp;] (CCallHelpers&amp; jit, const StackmapGenerationParams&amp; params) {
+            CHECK(params.reps.size() == 3);
+            CHECK(params.reps[0].isGPR());
+            CHECK(params.reps[1].isGPR());
+            CHECK(params.reps[2].isGPR());
+            jit.move(params.reps[1].gpr(), params.reps[0].gpr());
+            jit.add64(params.reps[2].gpr(), params.reps[0].gpr());
+
+            clobberAll.forEach([&amp;] (Reg reg) {
+                jit.move(CCallHelpers::TrustedImm32(0x00ff00ff), reg.gpr());
+            });
+        });
+
+    Value* result = root-&gt;appendNew&lt;Value&gt;(proc, Add, Origin(), patchpoint,
+        root-&gt;appendNew&lt;Value&gt;(proc, Add, Origin(), arg1, arg2));
+    root-&gt;appendNew&lt;ControlValue&gt;(proc, Return, Origin(), result);
+
+    CHECK(compileAndRun&lt;int&gt;(proc, 1, 2) == 58);
+}
+
+void testSimplePatchpointWithoutOuputClobbersFPArgs()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), FPRInfo::argumentFPR0);
+    Value* arg2 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), FPRInfo::argumentFPR1);
+    Value* const1 = root-&gt;appendNew&lt;ConstDoubleValue&gt;(proc, Origin(), 42.5);
+    Value* const2 = root-&gt;appendNew&lt;ConstDoubleValue&gt;(proc, Origin(), 13.1);
+
+    PatchpointValue* patchpoint = root-&gt;appendNew&lt;PatchpointValue&gt;(proc, Void, Origin());
+    patchpoint-&gt;clobber(RegisterSet(FPRInfo::argumentFPR0, FPRInfo::argumentFPR1));
+    patchpoint-&gt;append(ConstrainedValue(const1, ValueRep::SomeRegister));
+    patchpoint-&gt;append(ConstrainedValue(const2, ValueRep::SomeRegister));
+    patchpoint-&gt;setGenerator(
+        [&amp;] (CCallHelpers&amp; jit, const StackmapGenerationParams&amp; params) {
+            CHECK(params.reps.size() == 2);
+            CHECK(params.reps[0].isFPR());
+            CHECK(params.reps[1].isFPR());
+            jit.moveZeroToDouble(params.reps[0].fpr());
+            jit.moveZeroToDouble(params.reps[1].fpr());
+            jit.moveZeroToDouble(FPRInfo::argumentFPR0);
+            jit.moveZeroToDouble(FPRInfo::argumentFPR1);
+        });
+
+    Value* result = root-&gt;appendNew&lt;Value&gt;(proc, Add, Origin(), arg1, arg2);
+    root-&gt;appendNew&lt;ControlValue&gt;(proc, Return, Origin(), result);
+
+    CHECK(compileAndRun&lt;double&gt;(proc, 1.5, 2.5) == 4);
+}
+
+void testSimplePatchpointWithOuputClobbersFPArgs()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), FPRInfo::argumentFPR0);
+    Value* arg2 = root-&gt;appendNew&lt;ArgumentRegValue&gt;(proc, Origin(), FPRInfo::argumentFPR1);
+    Value* const1 = root-&gt;appendNew&lt;ConstDoubleValue&gt;(proc, Origin(), 42.5);
+    Value* const2 = root-&gt;appendNew&lt;ConstDoubleValue&gt;(proc, Origin(), 13.1);
+
+    PatchpointValue* patchpoint = root-&gt;appendNew&lt;PatchpointValue&gt;(proc, Double, Origin());
+
+    RegisterSet clobberAll = RegisterSet::allFPRs();
+    clobberAll.exclude(RegisterSet::stackRegisters());
+    clobberAll.exclude(RegisterSet::reservedHardwareRegisters());
+    clobberAll.clear(FPRInfo::argumentFPR2);
+    patchpoint-&gt;clobber(clobberAll);
+
+    patchpoint-&gt;append(ConstrainedValue(const1, ValueRep::SomeRegister));
+    patchpoint-&gt;append(ConstrainedValue(const2, ValueRep::SomeRegister));
+
+    patchpoint-&gt;setGenerator(
+        [&amp;] (CCallHelpers&amp; jit, const StackmapGenerationParams&amp; params) {
+            CHECK(params.reps.size() == 3);
+            CHECK(params.reps[0].isFPR());
+            CHECK(params.reps[1].isFPR());
+            CHECK(params.reps[2].isFPR());
+            jit.addDouble(params.reps[1].fpr(), params.reps[2].fpr(), params.reps[0].fpr());
+
+            clobberAll.forEach([&amp;] (Reg reg) {
+                jit.moveZeroToDouble(reg.fpr());
+            });
+        });
+
+    Value* result = root-&gt;appendNew&lt;Value&gt;(proc, Add, Origin(), patchpoint,
+        root-&gt;appendNew&lt;Value&gt;(proc, Add, Origin(), arg1, arg2));
+    root-&gt;appendNew&lt;ControlValue&gt;(proc, Return, Origin(), result);
+
+    CHECK(compileAndRun&lt;double&gt;(proc, 1.5, 2.5) == 59.6);
+}
+
</ins><span class="cx"> void testPatchpointCallArg()
</span><span class="cx"> {
</span><span class="cx">     Procedure proc;
</span><span class="lines">@@ -4516,6 +4664,10 @@
</span><span class="cx">     RUN(testComplex(4, 384));
</span><span class="cx"> 
</span><span class="cx">     RUN(testSimplePatchpoint());
</span><ins>+    RUN(testSimplePatchpointWithoutOuputClobbersGPArgs());
+    RUN(testSimplePatchpointWithOuputClobbersGPArgs());
+    RUN(testSimplePatchpointWithoutOuputClobbersFPArgs());
+    RUN(testSimplePatchpointWithOuputClobbersFPArgs());
</ins><span class="cx">     RUN(testPatchpointCallArg());
</span><span class="cx">     RUN(testPatchpointFixedRegister());
</span><span class="cx">     RUN(testPatchpointAny());
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRegisterSeth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/RegisterSet.h (192496 => 192497)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/RegisterSet.h        2015-11-17 00:26:57 UTC (rev 192496)
+++ trunk/Source/JavaScriptCore/jit/RegisterSet.h        2015-11-17 00:44:28 UTC (rev 192497)
</span><span class="lines">@@ -45,8 +45,8 @@
</span><span class="cx">         setMany(regs...);
</span><span class="cx">     }
</span><span class="cx">     
</span><del>-    static RegisterSet stackRegisters();
-    static RegisterSet reservedHardwareRegisters();
</del><ins>+    JS_EXPORT_PRIVATE static RegisterSet stackRegisters();
+    JS_EXPORT_PRIVATE static RegisterSet reservedHardwareRegisters();
</ins><span class="cx">     static RegisterSet runtimeRegisters();
</span><span class="cx">     static RegisterSet specialRegisters(); // The union of stack, reserved hardware, and runtime registers.
</span><span class="cx">     static RegisterSet calleeSaveRegisters();
</span><span class="lines">@@ -59,8 +59,8 @@
</span><span class="cx"> #endif
</span><span class="cx">     static RegisterSet volatileRegistersForJSCall();
</span><span class="cx">     static RegisterSet stubUnavailableRegisters(); // The union of callee saves and special registers.
</span><del>-    static RegisterSet allGPRs();
-    static RegisterSet allFPRs();
</del><ins>+    JS_EXPORT_PRIVATE static RegisterSet allGPRs();
+    JS_EXPORT_PRIVATE static RegisterSet allFPRs();
</ins><span class="cx">     static RegisterSet allRegisters();
</span><span class="cx"> 
</span><span class="cx">     static RegisterSet registersToNotSaveForJSCall();
</span></span></pre>
</div>
</div>

</body>
</html>