<!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>[183406] 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/183406">183406</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-04-27 11:49:15 -0700 (Mon, 27 Apr 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>VarargsForwardingPhase should use bytecode liveness in addition to other uses to determine the last point that a candidate is used
https://bugs.webkit.org/show_bug.cgi?id=143843
Reviewed by Geoffrey Garen.
It will soon come to pass that Phantom isn't available at the time that
VarargsForwardingPhase runs. So, it needs to use some other mechanism for discovering when
a value dies for OSR.
This is simplified by two things:
1) The bytecode kill analysis is now reusable. This patch makes it even more reusable than
before by polishing the API.
2) This phase already operates on one node at a time and allows itself to do a full search
of the enclosing basic block for that node. This is fine because CreateDirectArguments
and friends is a rarely occurring node. The fact that it operates on one node at a time
makes it even easier to reason about OSR liveness - we just track the list of locals in
which it is live.
This change has no effect right now but it is a necessary prerequisite to implementing
https://bugs.webkit.org/show_bug.cgi?id=143736.
* dfg/DFGBasicBlock.h:
(JSC::DFG::BasicBlock::tryAt):
* dfg/DFGForAllKills.h:
(JSC::DFG::forAllKilledOperands):
* dfg/DFGPhantomInsertionPhase.cpp:
* dfg/DFGVarargsForwardingPhase.cpp:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGBasicBlockh">trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGForAllKillsh">trunk/Source/JavaScriptCore/dfg/DFGForAllKills.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGPhantomInsertionPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGPhantomInsertionPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGVarargsForwardingPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (183405 => 183406)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-04-27 18:46:15 UTC (rev 183405)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-04-27 18:49:15 UTC (rev 183406)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2015-04-25 Filip Pizlo <fpizlo@apple.com>
+
+ VarargsForwardingPhase should use bytecode liveness in addition to other uses to determine the last point that a candidate is used
+ https://bugs.webkit.org/show_bug.cgi?id=143843
+
+ Reviewed by Geoffrey Garen.
+
+ It will soon come to pass that Phantom isn't available at the time that
+ VarargsForwardingPhase runs. So, it needs to use some other mechanism for discovering when
+ a value dies for OSR.
+
+ This is simplified by two things:
+
+ 1) The bytecode kill analysis is now reusable. This patch makes it even more reusable than
+ before by polishing the API.
+
+ 2) This phase already operates on one node at a time and allows itself to do a full search
+ of the enclosing basic block for that node. This is fine because CreateDirectArguments
+ and friends is a rarely occurring node. The fact that it operates on one node at a time
+ makes it even easier to reason about OSR liveness - we just track the list of locals in
+ which it is live.
+
+ This change has no effect right now but it is a necessary prerequisite to implementing
+ https://bugs.webkit.org/show_bug.cgi?id=143736.
+
+ * dfg/DFGBasicBlock.h:
+ (JSC::DFG::BasicBlock::tryAt):
+ * dfg/DFGForAllKills.h:
+ (JSC::DFG::forAllKilledOperands):
+ * dfg/DFGPhantomInsertionPhase.cpp:
+ * dfg/DFGVarargsForwardingPhase.cpp:
+
</ins><span class="cx"> 2015-04-27 Jordan Harband <ljharb@gmail.com>
</span><span class="cx">
</span><span class="cx"> Map#entries and Map#keys error for non-Maps is swapped
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGBasicBlockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h (183405 => 183406)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h 2015-04-27 18:46:15 UTC (rev 183405)
+++ trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h 2015-04-27 18:49:15 UTC (rev 183406)
</span><span class="lines">@@ -61,6 +61,12 @@
</span><span class="cx"> bool isEmpty() const { return !size(); }
</span><span class="cx"> Node*& at(size_t i) { return m_nodes[i]; }
</span><span class="cx"> Node* at(size_t i) const { return m_nodes[i]; }
</span><ins>+ Node* tryAt(size_t i) const
+ {
+ if (i >= size())
+ return nullptr;
+ return at(i);
+ }
</ins><span class="cx"> Node*& operator[](size_t i) { return at(i); }
</span><span class="cx"> Node* operator[](size_t i) const { return at(i); }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGForAllKillsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGForAllKills.h (183405 => 183406)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGForAllKills.h 2015-04-27 18:46:15 UTC (rev 183405)
+++ trunk/Source/JavaScriptCore/dfg/DFGForAllKills.h 2015-04-27 18:49:15 UTC (rev 183406)
</span><span class="lines">@@ -75,6 +75,12 @@
</span><span class="cx"> void forAllKilledOperands(Graph& graph, Node* nodeBefore, Node* nodeAfter, const Functor& functor)
</span><span class="cx"> {
</span><span class="cx"> CodeOrigin before = nodeBefore->origin.forExit;
</span><ins>+
+ if (!nodeAfter) {
+ graph.forAllLiveInBytecode(before, functor);
+ return;
+ }
+
</ins><span class="cx"> CodeOrigin after = nodeAfter->origin.forExit;
</span><span class="cx">
</span><span class="cx"> VirtualRegister alreadyNoted;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGPhantomInsertionPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGPhantomInsertionPhase.cpp (183405 => 183406)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGPhantomInsertionPhase.cpp 2015-04-27 18:46:15 UTC (rev 183405)
+++ trunk/Source/JavaScriptCore/dfg/DFGPhantomInsertionPhase.cpp 2015-04-27 18:49:15 UTC (rev 183406)
</span><span class="lines">@@ -122,42 +122,35 @@
</span><span class="cx">
</span><span class="cx"> node->setEpoch(currentEpoch);
</span><span class="cx">
</span><del>- auto killAction = [&] (VirtualRegister reg) {
- if (verbose)
- dataLog(" Killed operand: ", reg, "\n");
-
- Node* killedNode = m_values.operand(reg);
- if (!killedNode)
- return;
-
- // We only need to insert a Phantom if the node hasn't been used since the last
- // exit, and was born before the last exit.
- if (killedNode->epoch() == currentEpoch)
- return;
-
- if (verbose) {
- dataLog(
- " Inserting Phantom on ", killedNode, " after ",
- block->at(lastExitingIndex), "\n");
- }
-
- // We have exact ref counts, so creating a new use means that we have to increment
- // the ref count.
- killedNode->postfixRef();
-
- m_insertionSet.insertNode(
- lastExitingIndex + 1, SpecNone, Phantom, block->at(lastExitingIndex)->origin,
- killedNode->defaultEdge());
- };
-
- if (nodeIndex + 1 == block->size()) {
- // Should a MovHinted value be kept alive? If the value has been SetLocal'd then
- // the answer is no. But we may have a value that is live here and dead in
- // successors because we had jettisoned those successors that would have used the
- // value. Hence, anything live here should be kept alive.
- m_graph.forAllLiveInBytecode(node->origin.forExit, killAction);
- } else
- forAllKilledOperands(m_graph, node, block->at(nodeIndex + 1), killAction);
</del><ins>+ forAllKilledOperands(
+ m_graph, node, block->tryAt(nodeIndex + 1),
+ [&] (VirtualRegister reg) {
+ if (verbose)
+ dataLog(" Killed operand: ", reg, "\n");
+
+ Node* killedNode = m_values.operand(reg);
+ if (!killedNode)
+ return;
+
+ // We only need to insert a Phantom if the node hasn't been used since the last
+ // exit, and was born before the last exit.
+ if (killedNode->epoch() == currentEpoch)
+ return;
+
+ if (verbose) {
+ dataLog(
+ " Inserting Phantom on ", killedNode, " after ",
+ block->at(lastExitingIndex), "\n");
+ }
+
+ // We have exact ref counts, so creating a new use means that we have to
+ // increment the ref count.
+ killedNode->postfixRef();
+
+ m_insertionSet.insertNode(
+ lastExitingIndex + 1, SpecNone, Phantom,
+ block->at(lastExitingIndex)->origin, killedNode->defaultEdge());
+ });
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> m_insertionSet.execute(block);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGVarargsForwardingPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp (183405 => 183406)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp 2015-04-27 18:46:15 UTC (rev 183405)
+++ trunk/Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp 2015-04-27 18:49:15 UTC (rev 183406)
</span><span class="lines">@@ -30,6 +30,7 @@
</span><span class="cx">
</span><span class="cx"> #include "DFGArgumentsUtilities.h"
</span><span class="cx"> #include "DFGClobberize.h"
</span><ins>+#include "DFGForAllKills.h"
</ins><span class="cx"> #include "DFGGraph.h"
</span><span class="cx"> #include "DFGPhase.h"
</span><span class="cx"> #include "JSCInlines.h"
</span><span class="lines">@@ -49,6 +50,8 @@
</span><span class="cx">
</span><span class="cx"> bool run()
</span><span class="cx"> {
</span><ins>+ DFG_ASSERT(m_graph, nullptr, m_graph.m_form != SSA);
+
</ins><span class="cx"> if (verbose) {
</span><span class="cx"> dataLog("Graph before varargs forwarding:\n");
</span><span class="cx"> m_graph.dump();
</span><span class="lines">@@ -87,13 +90,19 @@
</span><span class="cx"> // Find the index of the last node in this block to use the candidate, and look for escaping
</span><span class="cx"> // sites.
</span><span class="cx"> unsigned lastUserIndex = candidateNodeIndex;
</span><ins>+ Vector<VirtualRegister, 2> relevantLocals; // This is a set. We expect it to be a small set.
</ins><span class="cx"> for (unsigned nodeIndex = candidateNodeIndex + 1; nodeIndex < block->size(); ++nodeIndex) {
</span><span class="cx"> Node* node = block->at(nodeIndex);
</span><ins>+
</ins><span class="cx"> switch (node->op()) {
</span><ins>+ case MovHint:
+ lastUserIndex = nodeIndex;
+ if (!relevantLocals.contains(node->unlinkedLocal()))
+ relevantLocals.append(node->unlinkedLocal());
+ break;
+
</ins><span class="cx"> case Phantom:
</span><span class="cx"> case Check:
</span><del>- case MovHint:
- case PutHint:
</del><span class="cx"> case LoadVarargs:
</span><span class="cx"> if (m_graph.uses(node, candidate))
</span><span class="cx"> lastUserIndex = nodeIndex;
</span><span class="lines">@@ -125,6 +134,18 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx"> }
</span><ins>+
+ forAllKilledOperands(
+ m_graph, node, block->tryAt(nodeIndex + 1),
+ [&] (VirtualRegister reg) {
+ for (unsigned i = 0; i < relevantLocals.size(); ++i) {
+ if (relevantLocals[i] == reg) {
+ relevantLocals[i--] = relevantLocals.last();
+ relevantLocals.removeLast();
+ lastUserIndex = nodeIndex;
+ }
+ }
+ });
</ins><span class="cx"> }
</span><span class="cx"> if (verbose)
</span><span class="cx"> dataLog("Selected lastUserIndex = ", lastUserIndex, ", ", block->at(lastUserIndex), "\n");
</span></span></pre>
</div>
</div>
</body>
</html>