<!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>[166281] 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/166281">166281</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2014-03-25 21:08:52 -0700 (Tue, 25 Mar 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>Arguments simplification phase should be fine with marking the arguments local itself as an arguments alias
https://bugs.webkit.org/show_bug.cgi?id=130764
&lt;rdar://problem/16304788&gt;

Reviewed by Sam Weinig.
        
Being an arguments alias just means that your OSR exit recovery should attempt arguments
creation. This is true of arguments locals. We had special cases that tried to make it not
true of arguments locals. The only consequence of those special cases was to cause crashes
in case of arguments that are also captured variables (i.e. we have SlowArguments). This
change just removes those special cases.
        
This change means that the FTL will now see SetLocals with a FlushedArguments format.
Previously you wouldn't see them because previously only non-captured variable would be
arguments aliases, and non-captured variables get completely SSAified - i.e. no SetLocals
left. Adding handling for FlushedArguments is a benign and simple change since its
behavior is identical to FlushedJSValue for that code's purposes.

* dfg/DFGArgumentsSimplificationPhase.cpp:
(JSC::DFG::ArgumentsSimplificationPhase::run):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileSetLocal):
* tests/stress/captured-arguments-variable.js: Added.
(foo):
(noInline):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGArgumentsSimplificationPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstresscapturedargumentsvariablejs">trunk/Source/JavaScriptCore/tests/stress/captured-arguments-variable.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (166280 => 166281)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2014-03-26 03:35:30 UTC (rev 166280)
+++ trunk/Source/JavaScriptCore/ChangeLog        2014-03-26 04:08:52 UTC (rev 166281)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2014-03-25  Filip Pizlo  &lt;fpizlo@apple.com&gt;
+
+        Arguments simplification phase should be fine with marking the arguments local itself as an arguments alias
+        https://bugs.webkit.org/show_bug.cgi?id=130764
+        &lt;rdar://problem/16304788&gt;
+
+        Reviewed by Sam Weinig.
+        
+        Being an arguments alias just means that your OSR exit recovery should attempt arguments
+        creation. This is true of arguments locals. We had special cases that tried to make it not
+        true of arguments locals. The only consequence of those special cases was to cause crashes
+        in case of arguments that are also captured variables (i.e. we have SlowArguments). This
+        change just removes those special cases.
+        
+        This change means that the FTL will now see SetLocals with a FlushedArguments format.
+        Previously you wouldn't see them because previously only non-captured variable would be
+        arguments aliases, and non-captured variables get completely SSAified - i.e. no SetLocals
+        left. Adding handling for FlushedArguments is a benign and simple change since its
+        behavior is identical to FlushedJSValue for that code's purposes.
+
+        * dfg/DFGArgumentsSimplificationPhase.cpp:
+        (JSC::DFG::ArgumentsSimplificationPhase::run):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileSetLocal):
+        * tests/stress/captured-arguments-variable.js: Added.
+        (foo):
+        (noInline):
+
</ins><span class="cx"> 2014-03-25  Mark Hahnenberg  &lt;mhahnenberg@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Add HeapInlines
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGArgumentsSimplificationPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp (166280 => 166281)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp        2014-03-26 03:35:30 UTC (rev 166280)
+++ trunk/Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp        2014-03-26 04:08:52 UTC (rev 166281)
</span><span class="lines">@@ -397,10 +397,6 @@
</span><span class="cx">                     
</span><span class="cx">                     VariableAccessData* variableAccessData = node-&gt;variableAccessData();
</span><span class="cx">                     
</span><del>-                    if (m_graph.argumentsRegisterFor(node-&gt;origin.semantic) == variableAccessData-&gt;local()
-                        || unmodifiedArgumentsRegister(m_graph.argumentsRegisterFor(node-&gt;origin.semantic)) == variableAccessData-&gt;local())
-                        break;
-
</del><span class="cx">                     if (variableAccessData-&gt;mergeIsArgumentsAlias(true)) {
</span><span class="cx">                         changed = true;
</span><span class="cx">                         
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (166280 => 166281)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp        2014-03-26 03:35:30 UTC (rev 166280)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp        2014-03-26 04:08:52 UTC (rev 166281)
</span><span class="lines">@@ -806,7 +806,8 @@
</span><span class="cx">     {
</span><span class="cx">         VariableAccessData* variable = m_node-&gt;variableAccessData();
</span><span class="cx">         switch (variable-&gt;flushFormat()) {
</span><del>-        case FlushedJSValue: {
</del><ins>+        case FlushedJSValue:
+        case FlushedArguments: {
</ins><span class="cx">             LValue value = lowJSValue(m_node-&gt;child1());
</span><span class="cx">             m_out.store64(value, addressFor(variable-&gt;machineLocal()));
</span><span class="cx">             break;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstresscapturedargumentsvariablejs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/captured-arguments-variable.js (0 => 166281)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/captured-arguments-variable.js                                (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/captured-arguments-variable.js        2014-03-26 04:08:52 UTC (rev 166281)
</span><span class="lines">@@ -0,0 +1,17 @@
</span><ins>+function foo(a) {
+    return arguments[1] + (function() { return a * 101; })();
+}
+
+noInline(foo);
+
+for (var i = 0; i &lt; 10000; ++i) {
+    var result = foo(42, 97);
+    if (result != 4339)
+        throw &quot;Error: bad result: &quot; + result;
+}
+
+Object.prototype[1] = 111;
+
+var result = foo(42);
+if (result != 4353)
+    throw &quot;Error: bad result at end: &quot; + result;
</ins></span></pre>
</div>
</div>

</body>
</html>