<!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>[203034] 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/203034">203034</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-07-09 14:07:45 -0700 (Sat, 09 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION(201900): validation failure for GetByOffset/PutByOffset in VALIDATE((node), node->child1().node() == node->child2().node() || node->child1()->result() == NodeResultStorage)
https://bugs.webkit.org/show_bug.cgi?id=159603
Reviewed by Keith Miller.
This removes an incorrect validation rule and replaces it with a FIXME about how to make this
aspect of IR easier to validate soundly.
It's not valid to assert that two children of a node are the same. It should always be valid
to take:
Foo(@x, @x)
and turn it into:
a: ValueRep(@x)
b: ValueRep(@x)
Foo(@a, @b)
or even something like:
y: Identity(@y)
Foo(@x, @y)
That's because it should be possible to rewire any data flow edge something that produces an
equivalent value.
The validation rule that this patch removes meant that such rewirings were invalid on
GetByOffset/PutByOffset. FixupPhase did such a rewiring sometimes.
* dfg/DFGValidate.cpp:
* tests/stress/get-by-offset-double.js: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGValidatecpp">trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressgetbyoffsetdoublejs">trunk/Source/JavaScriptCore/tests/stress/get-by-offset-double.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (203033 => 203034)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-07-09 20:50:51 UTC (rev 203033)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-07-09 21:07:45 UTC (rev 203034)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2016-07-09 Filip Pizlo <fpizlo@apple.com>
+
+ REGRESSION(201900): validation failure for GetByOffset/PutByOffset in VALIDATE((node), node->child1().node() == node->child2().node() || node->child1()->result() == NodeResultStorage)
+ https://bugs.webkit.org/show_bug.cgi?id=159603
+
+ Reviewed by Keith Miller.
+
+ This removes an incorrect validation rule and replaces it with a FIXME about how to make this
+ aspect of IR easier to validate soundly.
+
+ It's not valid to assert that two children of a node are the same. It should always be valid
+ to take:
+
+ Foo(@x, @x)
+
+ and turn it into:
+
+ a: ValueRep(@x)
+ b: ValueRep(@x)
+ Foo(@a, @b)
+
+ or even something like:
+
+ y: Identity(@y)
+ Foo(@x, @y)
+
+ That's because it should be possible to rewire any data flow edge something that produces an
+ equivalent value.
+
+ The validation rule that this patch removes meant that such rewirings were invalid on
+ GetByOffset/PutByOffset. FixupPhase did such a rewiring sometimes.
+
+ * dfg/DFGValidate.cpp:
+ * tests/stress/get-by-offset-double.js: Added.
+
</ins><span class="cx"> 2016-07-09 Keith Miller <keith_miller@apple.com>
</span><span class="cx">
</span><span class="cx"> appendMemcpy might fail in concatAppendOne
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGValidatecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp (203033 => 203034)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp 2016-07-09 20:50:51 UTC (rev 203033)
+++ trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp 2016-07-09 21:07:45 UTC (rev 203034)
</span><span class="lines">@@ -299,7 +299,12 @@
</span><span class="cx"> break;
</span><span class="cx"> case GetByOffset:
</span><span class="cx"> case PutByOffset:
</span><del>- VALIDATE((node), node->child1().node() == node->child2().node() || node->child1()->result() == NodeResultStorage);
</del><ins>+ // FIXME: We should be able to validate that GetByOffset and PutByOffset are
+ // using the same object for storage and base. I think this means finally
+ // splitting these nodes into two node types, one for inline and one for
+ // out-of-line. The out-of-line one will require that the first node is storage,
+ // while the inline one will not take a storage child at all.
+ // https://bugs.webkit.org/show_bug.cgi?id=159602
</ins><span class="cx"> break;
</span><span class="cx"> default:
</span><span class="cx"> break;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressgetbyoffsetdoublejs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/get-by-offset-double.js (0 => 203034)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/get-by-offset-double.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/get-by-offset-double.js 2016-07-09 21:07:45 UTC (rev 203034)
</span><span class="lines">@@ -0,0 +1,25 @@
</span><ins>+function foo(o, p) {
+ if (p)
+ return o.f;
+ else
+ return [o * 1.1, o * 1.2, o * 1.3];
+}
+
+for (var i = 0; i < 100; ++i)
+ foo({f:42}, true);
+
+function bar() {
+ var x = 4.5;
+ for (var i = 0; i < 10; ++i) {
+ x *= 1.1;
+ x += 0.05;
+ foo(x, false);
+ }
+ return x * 1.03;
+}
+
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i)
+ bar();
+
</ins></span></pre>
</div>
</div>
</body>
</html>