<!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>[174165] 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/174165">174165</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2014-10-01 13:12:11 -0700 (Wed, 01 Oct 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>DFG SSA should use PutLocal/KillLocal instead of SetLocal to communicate what is flushed to the stack and when
https://bugs.webkit.org/show_bug.cgi?id=137242
Reviewed by Geoffrey Garen.
OSR availability has to do with telling you the various ways that you could go about getting
the value of a bytecode variable. It can give you two options: node availability means that
there is a node in the DFG IR that has the right value, and flush availability tells you
that the value was already stored to the stack. The clients of OSR availability would
typically prefer flush over node availability.
Previously OSR availability was affected thusly by the various local-related nodes: SetLocal
set both the node and flush availability, MovHint set node availability and cleared flush
availability, GetArgument set both, and ZombieHint cleared both.
A MovHint could be turned into a ZombieHint if its source value was DCEd.
The fact that each node affected both node and flush availability caused weirdness. For
example it meant that we could not insert MovHints in areas of the CFG where a SetLocal's
variable was still live, because then those parts of the code would forget that they had an
availability flush. This meant that if a flush was available, we wouldn't insert MovHints,
and so we would forget that a node was in fact available. This kind of "either-or" picking
was not only hackish but it led to interesting problems for IR transformation: for example
if you tried to do any kind of code motion on SetLocals, you had to be super careful because
you might violate the rule that "MovHints must exist for a live local if a flush is
unavailable".
The right thing to do is to have independent nodes for flushing and making nodes available.
They shouldn't interact with each other. This patch accomplishes this:
- PutLocal means that that a value is to be stored to the stack. It makes a flush available.
- KillLocal means that the value stored to the stack is no longer available for the purposes
of OSR (i.e. it no longer accurately corresponds to what that actual bytecode variable
would have been, so you have to fall back on node availability).
- MovHint means that a node is available. It has no effect on flush availability.
- ZombieHint means that a node is not available. It has no effect on flush availability.
This means that we will see a lot of KillLocals and MovHints right next to each other. It's
a bit verbose, but at least it's precise.
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGAvailability.h:
(JSC::DFG::Availability::setFlush):
(JSC::DFG::Availability::setNode):
(JSC::DFG::Availability::setNodeUnavailable):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGNode.cpp:
(JSC::DFG::Node::hasVariableAccessData):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasUnlinkedLocal):
(JSC::DFG::Node::willHaveCodeGenOrOSR):
* dfg/DFGNodeType.h:
* dfg/DFGOSRAvailabilityAnalysisPhase.cpp:
(JSC::DFG::LocalOSRAvailabilityCalculator::executeNode):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSSAConversionPhase.cpp:
(JSC::DFG::SSAConversionPhase::run):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGStackLayoutPhase.cpp:
(JSC::DFG::StackLayoutPhase::run):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compilePutLocal):
(JSC::FTL::LowerDFGToLLVM::compileSetLocal): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGAbstractInterpreterInlinesh">trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGAvailabilityh">trunk/Source/JavaScriptCore/dfg/DFGAvailability.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGClobberizeh">trunk/Source/JavaScriptCore/dfg/DFGClobberize.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGDoesGCcpp">trunk/Source/JavaScriptCore/dfg/DFGDoesGC.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGFixupPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGNodecpp">trunk/Source/JavaScriptCore/dfg/DFGNode.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGNodeh">trunk/Source/JavaScriptCore/dfg/DFGNode.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGNodeTypeh">trunk/Source/JavaScriptCore/dfg/DFGNodeType.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGOSRAvailabilityAnalysisPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGPredictionPropagationPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSSAConversionPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSafeToExecuteh">trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGStackLayoutPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLCapabilitiescpp">trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/ChangeLog 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -1,3 +1,82 @@
</span><ins>+2014-09-30 Filip Pizlo <fpizlo@apple.com>
+
+ DFG SSA should use PutLocal/KillLocal instead of SetLocal to communicate what is flushed to the stack and when
+ https://bugs.webkit.org/show_bug.cgi?id=137242
+
+ Reviewed by Geoffrey Garen.
+
+ OSR availability has to do with telling you the various ways that you could go about getting
+ the value of a bytecode variable. It can give you two options: node availability means that
+ there is a node in the DFG IR that has the right value, and flush availability tells you
+ that the value was already stored to the stack. The clients of OSR availability would
+ typically prefer flush over node availability.
+
+ Previously OSR availability was affected thusly by the various local-related nodes: SetLocal
+ set both the node and flush availability, MovHint set node availability and cleared flush
+ availability, GetArgument set both, and ZombieHint cleared both.
+
+ A MovHint could be turned into a ZombieHint if its source value was DCEd.
+
+ The fact that each node affected both node and flush availability caused weirdness. For
+ example it meant that we could not insert MovHints in areas of the CFG where a SetLocal's
+ variable was still live, because then those parts of the code would forget that they had an
+ availability flush. This meant that if a flush was available, we wouldn't insert MovHints,
+ and so we would forget that a node was in fact available. This kind of "either-or" picking
+ was not only hackish but it led to interesting problems for IR transformation: for example
+ if you tried to do any kind of code motion on SetLocals, you had to be super careful because
+ you might violate the rule that "MovHints must exist for a live local if a flush is
+ unavailable".
+
+ The right thing to do is to have independent nodes for flushing and making nodes available.
+ They shouldn't interact with each other. This patch accomplishes this:
+
+ - PutLocal means that that a value is to be stored to the stack. It makes a flush available.
+ - KillLocal means that the value stored to the stack is no longer available for the purposes
+ of OSR (i.e. it no longer accurately corresponds to what that actual bytecode variable
+ would have been, so you have to fall back on node availability).
+ - MovHint means that a node is available. It has no effect on flush availability.
+ - ZombieHint means that a node is not available. It has no effect on flush availability.
+
+ This means that we will see a lot of KillLocals and MovHints right next to each other. It's
+ a bit verbose, but at least it's precise.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGAvailability.h:
+ (JSC::DFG::Availability::setFlush):
+ (JSC::DFG::Availability::setNode):
+ (JSC::DFG::Availability::setNodeUnavailable):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGDoesGC.cpp:
+ (JSC::DFG::doesGC):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGNode.cpp:
+ (JSC::DFG::Node::hasVariableAccessData):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::hasUnlinkedLocal):
+ (JSC::DFG::Node::willHaveCodeGenOrOSR):
+ * dfg/DFGNodeType.h:
+ * dfg/DFGOSRAvailabilityAnalysisPhase.cpp:
+ (JSC::DFG::LocalOSRAvailabilityCalculator::executeNode):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ (JSC::DFG::PredictionPropagationPhase::propagate):
+ * dfg/DFGSSAConversionPhase.cpp:
+ (JSC::DFG::SSAConversionPhase::run):
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::safeToExecute):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGStackLayoutPhase.cpp:
+ (JSC::DFG::StackLayoutPhase::run):
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::compileNode):
+ (JSC::FTL::LowerDFGToLLVM::compilePutLocal):
+ (JSC::FTL::LowerDFGToLLVM::compileSetLocal): Deleted.
+
</ins><span class="cx"> 2014-10-01 Brent Fulgham <bfulgham@apple.com>
</span><span class="cx">
</span><span class="cx"> [Win] 32-bit JavaScriptCore should limit itself to the C loop
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGAbstractInterpreterInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -183,7 +183,8 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- case SetLocal: {
</del><ins>+ case SetLocal:
+ case PutLocal: {
</ins><span class="cx"> m_state.variables().operand(node->local().offset()) = forNode(node->child1());
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="lines">@@ -195,6 +196,12 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ case KillLocal: {
+ // This is just a hint telling us that the OSR state of the local is no longer inside the
+ // flushed data.
+ break;
+ }
+
</ins><span class="cx"> case SetArgument:
</span><span class="cx"> // Assert that the state of arguments has been set.
</span><span class="cx"> ASSERT(!m_state.block()->valuesAtHead.operand(node->local()).isClear());
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGAvailabilityh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGAvailability.h (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGAvailability.h 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGAvailability.h 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -81,6 +81,21 @@
</span><span class="cx"> return withNode(unavailableMarker());
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ void setFlush(FlushedAt flushedAt)
+ {
+ m_flushedAt = flushedAt;
+ }
+
+ void setNode(Node* node)
+ {
+ m_node = node;
+ }
+
+ void setNodeUnavailable()
+ {
+ m_node = unavailableMarker();
+ }
+
</ins><span class="cx"> bool nodeIsUndecided() const { return !m_node; }
</span><span class="cx"> bool nodeIsUnavailable() const { return m_node == unavailableMarker(); }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGClobberizeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGClobberize.h (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGClobberize.h 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGClobberize.h 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -252,6 +252,7 @@
</span><span class="cx">
</span><span class="cx"> case MovHint:
</span><span class="cx"> case ZombieHint:
</span><ins>+ case KillLocal:
</ins><span class="cx"> case Upsilon:
</span><span class="cx"> case Phi:
</span><span class="cx"> case PhantomLocal:
</span><span class="lines">@@ -395,6 +396,7 @@
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> case SetLocal:
</span><ins>+ case PutLocal:
</ins><span class="cx"> write(AbstractHeap(Variables, node->local()));
</span><span class="cx"> def(HeapLocation(VariableLoc, AbstractHeap(Variables, node->local())), node->child1().node());
</span><span class="cx"> return;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGDoesGCcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGDoesGC.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGDoesGC.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGDoesGC.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -203,6 +203,8 @@
</span><span class="cx"> case PutByOffsetHint:
</span><span class="cx"> case CheckStructureImmediate:
</span><span class="cx"> case PutStructureHint:
</span><ins>+ case PutLocal:
+ case KillLocal:
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> case CreateActivation:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGFixupPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -1018,6 +1018,8 @@
</span><span class="cx"> case CheckStructureImmediate:
</span><span class="cx"> case PutStructureHint:
</span><span class="cx"> case MaterializeNewObject:
</span><ins>+ case PutLocal:
+ case KillLocal:
</ins><span class="cx"> // These are just nodes that we don't currently expect to see during fixup.
</span><span class="cx"> // If we ever wanted to insert them prior to fixup, then we just have to create
</span><span class="cx"> // fixup rules for them.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGNodecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGNode.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGNode.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGNode.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -79,6 +79,7 @@
</span><span class="cx"> case SetArgument:
</span><span class="cx"> case Flush:
</span><span class="cx"> case PhantomLocal:
</span><ins>+ case PutLocal:
</ins><span class="cx"> return true;
</span><span class="cx"> default:
</span><span class="cx"> return false;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGNodeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGNode.h (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGNode.h 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGNode.h 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -683,6 +683,7 @@
</span><span class="cx"> case ExtractOSREntryLocal:
</span><span class="cx"> case MovHint:
</span><span class="cx"> case ZombieHint:
</span><ins>+ case KillLocal:
</ins><span class="cx"> return true;
</span><span class="cx"> default:
</span><span class="cx"> return false;
</span><span class="lines">@@ -1390,6 +1391,8 @@
</span><span class="cx"> {
</span><span class="cx"> switch (op()) {
</span><span class="cx"> case SetLocal:
</span><ins>+ case PutLocal:
+ case KillLocal:
</ins><span class="cx"> case MovHint:
</span><span class="cx"> case ZombieHint:
</span><span class="cx"> case PhantomArguments:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGNodeTypeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGNodeType.h (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGNodeType.h 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGNodeType.h 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -53,9 +53,12 @@
</span><span class="cx"> \
</span><span class="cx"> /* Nodes for local variable access. These nodes are linked together using Phi nodes. */\
</span><span class="cx"> /* Any two nodes that are part of the same Phi graph will share the same */\
</span><del>- /* VariableAccessData, and thus will share predictions. */\
</del><ins>+ /* VariableAccessData, and thus will share predictions. FIXME: We should come up with */\
+ /* better names for a lot of these. https://bugs.webkit.org/show_bug.cgi?id=137307 */\
</ins><span class="cx"> macro(GetLocal, NodeResultJS) \
</span><span class="cx"> macro(SetLocal, 0) \
</span><ins>+ macro(PutLocal, NodeMustGenerate) \
+ macro(KillLocal, NodeMustGenerate) \
</ins><span class="cx"> macro(MovHint, 0) \
</span><span class="cx"> macro(ZombieHint, 0) \
</span><span class="cx"> macro(GetArgument, NodeResultJS | NodeMustGenerate) \
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGOSRAvailabilityAnalysisPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -127,12 +127,16 @@
</span><span class="cx"> void LocalOSRAvailabilityCalculator::executeNode(Node* node)
</span><span class="cx"> {
</span><span class="cx"> switch (node->op()) {
</span><del>- case SetLocal: {
</del><ins>+ case PutLocal: {
</ins><span class="cx"> VariableAccessData* variable = node->variableAccessData();
</span><del>- m_availability.m_locals.operand(variable->local()) =
- Availability(node->child1().node(), variable->flushedAt());
</del><ins>+ m_availability.m_locals.operand(variable->local()).setFlush(variable->flushedAt());
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><ins>+
+ case KillLocal: {
+ m_availability.m_locals.operand(node->unlinkedLocal()).setFlush(FlushedAt(ConflictingFlush));
+ break;
+ }
</ins><span class="cx">
</span><span class="cx"> case GetArgument: {
</span><span class="cx"> VariableAccessData* variable = node->variableAccessData();
</span><span class="lines">@@ -142,14 +146,12 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case MovHint: {
</span><del>- m_availability.m_locals.operand(node->unlinkedLocal()) =
- Availability(node->child1().node());
</del><ins>+ m_availability.m_locals.operand(node->unlinkedLocal()).setNode(node->child1().node());
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case ZombieHint: {
</span><del>- m_availability.m_locals.operand(node->unlinkedLocal()) =
- Availability::unavailable();
</del><ins>+ m_availability.m_locals.operand(node->unlinkedLocal()).setNodeUnavailable();
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGPredictionPropagationPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -565,7 +565,9 @@
</span><span class="cx"> case PutByOffsetHint:
</span><span class="cx"> case CheckStructureImmediate:
</span><span class="cx"> case PutStructureHint:
</span><del>- case MaterializeNewObject: {
</del><ins>+ case MaterializeNewObject:
+ case PutLocal:
+ case KillLocal: {
</ins><span class="cx"> // This node should never be visible at this stage of compilation. It is
</span><span class="cx"> // inserted by fixup(), which follows this phase.
</span><span class="cx"> RELEASE_ASSERT_NOT_REACHED();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSSAConversionPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -184,12 +184,14 @@
</span><span class="cx"> // - Convert all of the preexisting SSA nodes (other than the old CPS Phi nodes) into SSA
</span><span class="cx"> // form by replacing as follows:
</span><span class="cx"> //
</span><ins>+ // - MovHint has KillLocal prepended to it.
+ //
</ins><span class="cx"> // - GetLocal over captured variables lose their phis.
</span><span class="cx"> //
</span><span class="cx"> // - GetLocal over uncaptured variables die and get replaced with references to the node
</span><span class="cx"> // specified by valueForOperand.
</span><span class="cx"> //
</span><del>- // - SetLocal gets NodeMustGenerate if it's flushed, or turns into a Check otherwise.
</del><ins>+ // - SetLocal turns into PutLocal if it's flushed, or turns into a Check otherwise.
</ins><span class="cx"> //
</span><span class="cx"> // - Flush loses its children and turns into a Phantom.
</span><span class="cx"> //
</span><span class="lines">@@ -249,41 +251,12 @@
</span><span class="cx"> for (SSACalculator::Def* phiDef : m_calculator.phisForBlock(block)) {
</span><span class="cx"> VariableAccessData* variable = m_variableForSSAIndex[phiDef->variable()->index()];
</span><span class="cx">
</span><del>- // Figure out if the local is meant to be flushed here.
- bool isFlushed = !!(
- block->variablesAtHead.operand(variable->local())->flags() & NodeIsFlushed);
-
</del><span class="cx"> m_insertionSet.insert(phiInsertionPoint, phiDef->value());
</span><span class="cx"> valueForOperand.operand(variable->local()) = phiDef->value();
</span><span class="cx">
</span><del>- if (isFlushed) {
- // Do nothing. For multiple reasons.
-
- // Reason #1: If the local is flushed then we don't need to bother with a
- // MovHint since every path to this point in the code will have flushed the
- // bytecode variable using a SetLocal and hence the Availability::flushedAt()
- // will agree, and that will be sufficient for figuring out how to recover the
- // variable's value.
-
- // Reason #2: If we had inserted a MovHint and the Phi function had died
- // (because the only user of the value was the "flush" - i.e. some
- // asynchronous runtime thingy) then the MovHint would turn into a ZombieHint,
- // which would fool us into thinking that the variable is dead. Note that this
- // reason has a lot to do with the fact that Flushes do not turn into
- // Phantoms, because our handling of Flushes assume that we (1) always leave
- // the flushed SetLocals alone and (2) OSR availability analysis always sees
- // the available flush. The presence of a MovHint or ZombieHint would make the
- // flush seem unavailable.
-
- // Reason #3: If we had inserted a MovHint then even if the Phi stayed alive,
- // we would still end up generating inefficient code since we would be telling
- // the OSR exit compiler to use some SSA value for the bytecode variable
- // rather than just telling it that the value was already on the stack.
- } else {
- m_insertionSet.insertNode(
- phiInsertionPoint, SpecNone, MovHint, NodeOrigin(),
- OpInfo(variable->local().offset()), phiDef->value()->defaultEdge());
- }
</del><ins>+ m_insertionSet.insertNode(
+ phiInsertionPoint, SpecNone, MovHint, NodeOrigin(),
+ OpInfo(variable->local().offset()), phiDef->value()->defaultEdge());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
</span><span class="lines">@@ -297,11 +270,18 @@
</span><span class="cx"> m_graph.performSubstitution(node);
</span><span class="cx">
</span><span class="cx"> switch (node->op()) {
</span><ins>+ case MovHint: {
+ m_insertionSet.insertNode(
+ nodeIndex, SpecNone, KillLocal, node->origin,
+ OpInfo(node->unlinkedLocal().offset()));
+ break;
+ }
+
</ins><span class="cx"> case SetLocal: {
</span><span class="cx"> VariableAccessData* variable = node->variableAccessData();
</span><span class="cx">
</span><span class="cx"> if (variable->isCaptured() || !!(node->flags() & NodeIsFlushed))
</span><del>- node->mergeFlags(NodeMustGenerate);
</del><ins>+ node->setOpAndDefaultFlags(PutLocal);
</ins><span class="cx"> else
</span><span class="cx"> node->setOpAndDefaultFlags(Check);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSafeToExecuteh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -118,6 +118,8 @@
</span><span class="cx"> case GetCallee:
</span><span class="cx"> case GetLocal:
</span><span class="cx"> case SetLocal:
</span><ins>+ case PutLocal:
+ case KillLocal:
</ins><span class="cx"> case MovHint:
</span><span class="cx"> case ZombieHint:
</span><span class="cx"> case GetArgument:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -4985,6 +4985,8 @@
</span><span class="cx"> case CheckStructureImmediate:
</span><span class="cx"> case PutStructureHint:
</span><span class="cx"> case MaterializeNewObject:
</span><ins>+ case PutLocal:
+ case KillLocal:
</ins><span class="cx"> DFG_CRASH(m_jit.graph(), node, "Unexpected node");
</span><span class="cx"> break;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGStackLayoutPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -66,6 +66,7 @@
</span><span class="cx"> switch (node->op()) {
</span><span class="cx"> case GetLocal:
</span><span class="cx"> case SetLocal:
</span><ins>+ case PutLocal:
</ins><span class="cx"> case Flush:
</span><span class="cx"> case PhantomLocal: {
</span><span class="cx"> VariableAccessData* variable = node->variableAccessData();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLCapabilitiescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -47,6 +47,8 @@
</span><span class="cx"> case GetMyArgumentsLength:
</span><span class="cx"> case GetLocal:
</span><span class="cx"> case SetLocal:
</span><ins>+ case PutLocal:
+ case KillLocal:
</ins><span class="cx"> case MovHint:
</span><span class="cx"> case ZombieHint:
</span><span class="cx"> case Phantom:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (174164 => 174165)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2014-10-01 18:30:58 UTC (rev 174164)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2014-10-01 20:12:11 UTC (rev 174165)
</span><span class="lines">@@ -386,8 +386,8 @@
</span><span class="cx"> case GetLocal:
</span><span class="cx"> compileGetLocal();
</span><span class="cx"> break;
</span><del>- case SetLocal:
- compileSetLocal();
</del><ins>+ case PutLocal:
+ compilePutLocal();
</ins><span class="cx"> break;
</span><span class="cx"> case GetMyArgumentsLength:
</span><span class="cx"> compileGetMyArgumentsLength();
</span><span class="lines">@@ -746,6 +746,7 @@
</span><span class="cx"> case PutByOffsetHint:
</span><span class="cx"> case PutStructureHint:
</span><span class="cx"> case BottomValue:
</span><ins>+ case KillLocal:
</ins><span class="cx"> break;
</span><span class="cx"> default:
</span><span class="cx"> DFG_CRASH(m_graph, m_node, "Unrecognized node in FTL backend");
</span><span class="lines">@@ -1024,7 +1025,7 @@
</span><span class="cx"> setJSValue(m_out.load64(addressFor(variable->machineLocal())));
</span><span class="cx"> }
</span><span class="cx">
</span><del>- void compileSetLocal()
</del><ins>+ void compilePutLocal()
</ins><span class="cx"> {
</span><span class="cx"> VariableAccessData* variable = m_node->variableAccessData();
</span><span class="cx"> switch (variable->flushFormat()) {
</span></span></pre>
</div>
</div>
</body>
</html>