<!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>[204857] 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/204857">204857</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-08-23 13:43:56 -0700 (Tue, 23 Aug 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Spilling of constant tmps should make it easier for the spill code optimizer to rematerialize the constant
https://bugs.webkit.org/show_bug.cgi?id=160150
Reviewed by Benjamin Poulain.
When we spill in-place for admitsStack()==true, we prevent rematerialization if that
argument doesn't also admit immediates (which it almost certainly won't do). So, we
prevent remat.
This fixes the issue by avoiding in-place spilling for warm uses of constants. I don't
know if this helps performance, but I do know that it make the codegen for
bigswitch-indirect-symbol look a lot better. Prior to this change, the prolog would have
a constant materialization for each symbol that function used, and then it would spill
that constant. This removes all of that yucky code.
This also changes how IRC detects constant Tmps. Previously we would say that a Tmp is a
constant if the number of const defs was equal to the number of defs. But it's possible
for each of the const defs to produce a different value. This is unlikely considering
how B3->Air lowering works and how our SSA works - each def would have its own register.
But, regardless, this picks a more precise way of detecting constants: the number of
const defs must be 1 and the number of defs must be 1.
* 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 (204856 => 204857)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-08-23 20:29:35 UTC (rev 204856)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-08-23 20:43:56 UTC (rev 204857)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2016-07-24 Filip Pizlo <fpizlo@apple.com>
+
+ Spilling of constant tmps should make it easier for the spill code optimizer to rematerialize the constant
+ https://bugs.webkit.org/show_bug.cgi?id=160150
+
+ Reviewed by Benjamin Poulain.
+
+ When we spill in-place for admitsStack()==true, we prevent rematerialization if that
+ argument doesn't also admit immediates (which it almost certainly won't do). So, we
+ prevent remat.
+
+ This fixes the issue by avoiding in-place spilling for warm uses of constants. I don't
+ know if this helps performance, but I do know that it make the codegen for
+ bigswitch-indirect-symbol look a lot better. Prior to this change, the prolog would have
+ a constant materialization for each symbol that function used, and then it would spill
+ that constant. This removes all of that yucky code.
+
+ This also changes how IRC detects constant Tmps. Previously we would say that a Tmp is a
+ constant if the number of const defs was equal to the number of defs. But it's possible
+ for each of the const defs to produce a different value. This is unlikely considering
+ how B3->Air lowering works and how our SSA works - each def would have its own register.
+ But, regardless, this picks a more precise way of detecting constants: the number of
+ const defs must be 1 and the number of defs must be 1.
+
+ * b3/air/AirIteratedRegisterCoalescing.cpp:
+
</ins><span class="cx"> 2016-08-23 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed, fix CLoop build.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirIteratedRegisterCoalescingcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp (204856 => 204857)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp 2016-08-23 20:29:35 UTC (rev 204856)
+++ trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp 2016-08-23 20:43:56 UTC (rev 204857)
</span><span class="lines">@@ -1121,7 +1121,7 @@
</span><span class="cx">
</span><span class="cx"> // If it's a constant, then it's not as bad to spill. We can rematerialize it in many
</span><span class="cx"> // cases.
</span><del>- if (counts->numConstDefs == counts->numDefs)
</del><ins>+ if (counts->numConstDefs == 1 && counts->numDefs == 1)
</ins><span class="cx"> uses /= 2;
</span><span class="cx">
</span><span class="cx"> return degree / uses;
</span><span class="lines">@@ -1462,25 +1462,41 @@
</span><span class="cx"> // Try to replace the register use by memory use when possible.
</span><span class="cx"> inst.forEachArg(
</span><span class="cx"> [&] (Arg& arg, Arg::Role role, Arg::Type argType, Arg::Width width) {
</span><del>- if (arg.isTmp() && argType == type && !arg.isReg()) {
- auto stackSlotEntry = stackSlots.find(arg.tmp());
- if (stackSlotEntry != stackSlots.end()
- && inst.admitsStack(arg)) {
-
- Arg::Width spillWidth = m_tmpWidth.requiredWidth(arg.tmp());
- if (Arg::isAnyDef(role) && width < spillWidth)
- return;
- ASSERT(inst.opcode == Move || !(Arg::isAnyUse(role) && width > spillWidth));
-
- if (spillWidth != Arg::Width32)
- canUseMove32IfDidSpill = false;
-
- stackSlotEntry->value->ensureSize(
- canUseMove32IfDidSpill ? 4 : Arg::bytes(width));
- arg = Arg::stack(stackSlotEntry->value);
- didSpill = true;
- }
</del><ins>+ if (!arg.isTmp())
+ return;
+ if (argType != type)
+ return;
+ if (arg.isReg())
+ return;
+
+ auto stackSlotEntry = stackSlots.find(arg.tmp());
+ if (stackSlotEntry == stackSlots.end())
+ return;
+ if (!inst.admitsStack(arg))
+ return;
+
+ // If the Tmp holds a constant then we want to rematerialize its
+ // value rather than loading it from the stack. In order for that
+ // optimization to kick in, we need to avoid placing the Tmp's stack
+ // address into the instruction.
+ if (!Arg::isColdUse(role)) {
+ const UseCounts<Tmp>::Counts* counts = m_useCounts[arg.tmp()];
+ if (counts && counts->numConstDefs == 1 && counts->numDefs == 1)
+ return;
</ins><span class="cx"> }
</span><ins>+
+ Arg::Width spillWidth = m_tmpWidth.requiredWidth(arg.tmp());
+ if (Arg::isAnyDef(role) && width < spillWidth)
+ return;
+ ASSERT(inst.opcode == Move || !(Arg::isAnyUse(role) && width > spillWidth));
+
+ if (spillWidth != Arg::Width32)
+ canUseMove32IfDidSpill = false;
+
+ stackSlotEntry->value->ensureSize(
+ canUseMove32IfDidSpill ? 4 : Arg::bytes(width));
+ arg = Arg::stack(stackSlotEntry->value);
+ didSpill = true;
</ins><span class="cx"> });
</span><span class="cx">
</span><span class="cx"> if (didSpill && canUseMove32IfDidSpill)
</span></span></pre>
</div>
</div>
</body>
</html>