<!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>[181487] 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/181487">181487</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-03-13 13:18:08 -0700 (Fri, 13 Mar 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>DFG::PutStackSinkingPhase should eliminate GetStacks that have an obviously known source
https://bugs.webkit.org/show_bug.cgi?id=141624

Reviewed by Oliver Hunt.
        
This was an obvious omission from the original PutStackSinkingPhase. Previously, we would treat
GetStacks conservatively and assume that the stack slot escaped. That's pretty dumb, since a
GetStack is a local load of the stack. This change makes GetStack a no-op from the standpoint of
this phase's deferral analysis. At the end we either keep the GetStack (if there was no concrete
deferral) or we replace it with an identity over the value that would have been stored by the
deferred PutStack. Note that this might be a Phi that the phase creates, so this is strictly
stronger than what GCSE could do.
        
This is probably not a speed-up now, but it will be very useful for the varargs simplification
done in bug 141174.

* dfg/DFGPutStackSinkingPhase.cpp:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGPutStackSinkingPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (181486 => 181487)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2015-03-13 20:14:39 UTC (rev 181486)
+++ trunk/Source/JavaScriptCore/ChangeLog        2015-03-13 20:18:08 UTC (rev 181487)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2015-03-13  Filip Pizlo  &lt;fpizlo@apple.com&gt;
+
+        DFG::PutStackSinkingPhase should eliminate GetStacks that have an obviously known source
+        https://bugs.webkit.org/show_bug.cgi?id=141624
+
+        Reviewed by Oliver Hunt.
+        
+        This was an obvious omission from the original PutStackSinkingPhase. Previously, we would treat
+        GetStacks conservatively and assume that the stack slot escaped. That's pretty dumb, since a
+        GetStack is a local load of the stack. This change makes GetStack a no-op from the standpoint of
+        this phase's deferral analysis. At the end we either keep the GetStack (if there was no concrete
+        deferral) or we replace it with an identity over the value that would have been stored by the
+        deferred PutStack. Note that this might be a Phi that the phase creates, so this is strictly
+        stronger than what GCSE could do.
+        
+        This is probably not a speed-up now, but it will be very useful for the varargs simplification
+        done in bug 141174.
+
+        * dfg/DFGPutStackSinkingPhase.cpp:
+
</ins><span class="cx"> 2015-03-12  Geoffrey Garen  &lt;ggaren@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Prohibit GC while sweeping
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGPutStackSinkingPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp (181486 => 181487)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp        2015-03-13 20:14:39 UTC (rev 181486)
+++ trunk/Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp        2015-03-13 20:18:08 UTC (rev 181487)
</span><span class="lines">@@ -221,6 +221,12 @@
</span><span class="cx">                         continue;
</span><span class="cx">                     }
</span><span class="cx">                     
</span><ins>+                    if (node-&gt;op() == GetStack) {
+                        // A GetStack doesn't affect anything, since we know which local we are reading
+                        // from.
+                        continue;
+                    }
+                    
</ins><span class="cx">                     auto escapeHandler = [&amp;] (VirtualRegister operand) {
</span><span class="cx">                         if (operand.isHeader())
</span><span class="cx">                             return;
</span><span class="lines">@@ -390,6 +396,28 @@
</span><span class="cx">                     deferred.operand(node-&gt;unlinkedLocal()) = ConflictingFlush;
</span><span class="cx">                     break;
</span><span class="cx">                 }
</span><ins>+                    
+                case GetStack: {
+                    StackAccessData* data = node-&gt;stackAccessData();
+                    FlushFormat format = deferred.operand(data-&gt;local);
+                    if (!isConcrete(format)) {
+                        // This means there is no deferral. No deferral means that the most
+                        // authoritative value for this stack slot is what is stored in the stack. So,
+                        // keep the GetStack.
+                        break;
+                    }
+                    
+                    // We have a concrete deferral, which means a PutStack that hasn't executed yet. It
+                    // would have stored a value with a certain format. That format must match our
+                    // format. But more importantly, we can simply use the value that the PutStack would
+                    // have stored and get rid of the GetStack.
+                    DFG_ASSERT(m_graph, node, format == data-&gt;format);
+                    
+                    Node* incoming = mapping.operand(data-&gt;local);
+                    node-&gt;convertToIdentity();
+                    node-&gt;child1() = incoming-&gt;defaultEdge();
+                    break;
+                }
</ins><span class="cx">                 
</span><span class="cx">                 default: {
</span><span class="cx">                     auto escapeHandler = [&amp;] (VirtualRegister operand) {
</span><span class="lines">@@ -418,16 +446,6 @@
</span><span class="cx">                     preciseLocalClobberize(
</span><span class="cx">                         m_graph, node, escapeHandler, escapeHandler,
</span><span class="cx">                         [&amp;] (VirtualRegister, Node*) { });
</span><del>-                    
-                    // If we're a GetStack, then we also create a mapping.
-                    // FIXME: We should be able to just eliminate such GetLocals, when we know
-                    // what their incoming value will be.
-                    // https://bugs.webkit.org/show_bug.cgi?id=141624
-                    if (node-&gt;op() == GetStack) {
-                        StackAccessData* data = node-&gt;stackAccessData();
-                        VirtualRegister operand = data-&gt;local;
-                        mapping.operand(operand) = node;
-                    }
</del><span class="cx">                     break;
</span><span class="cx">                 } }
</span><span class="cx">             }
</span></span></pre>
</div>
</div>

</body>
</html>