<!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>[188357] 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/188357">188357</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-08-12 14:01:15 -0700 (Wed, 12 Aug 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>DFG::ByteCodeParser should attempt constant folding on loads from structures that are DFG-watchable
https://bugs.webkit.org/show_bug.cgi?id=147950

Reviewed by Michael Saboff.

Previously we reduced the constant folding power of ByteCodeParser::load() because that code was
responsible for memory corruption, since it would sometimes install watchpoints on structures that
weren't being traced.  It seemed like the safest fix was to remove the constant folding rule
entirely since later phases also do constant folding, and they do it without introducing the bug.
Well, that change (http://trac.webkit.org/changeset/188292) caused a big regression, because we
still have some constant folding rules that only exist in ByteCodeParser, and so ByteCodeParser must
be maximally aggressive in constant-folding whenever possible.

So, this change now brings back that constant folding rule - for loads from object constants that
have DFG-watchable structures - and implements it properly, by ensuring that we only call into
tryGetConstantProperty() if we have registered the structure set.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::load):</pre>

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

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (188356 => 188357)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2015-08-12 20:51:57 UTC (rev 188356)
+++ trunk/Source/JavaScriptCore/ChangeLog        2015-08-12 21:01:15 UTC (rev 188357)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2015-08-12  Filip Pizlo  &lt;fpizlo@apple.com&gt;
+
+        DFG::ByteCodeParser should attempt constant folding on loads from structures that are DFG-watchable
+        https://bugs.webkit.org/show_bug.cgi?id=147950
+
+        Reviewed by Michael Saboff.
+
+        Previously we reduced the constant folding power of ByteCodeParser::load() because that code was
+        responsible for memory corruption, since it would sometimes install watchpoints on structures that
+        weren't being traced.  It seemed like the safest fix was to remove the constant folding rule
+        entirely since later phases also do constant folding, and they do it without introducing the bug.
+        Well, that change (http://trac.webkit.org/changeset/188292) caused a big regression, because we
+        still have some constant folding rules that only exist in ByteCodeParser, and so ByteCodeParser must
+        be maximally aggressive in constant-folding whenever possible.
+
+        So, this change now brings back that constant folding rule - for loads from object constants that
+        have DFG-watchable structures - and implements it properly, by ensuring that we only call into
+        tryGetConstantProperty() if we have registered the structure set.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::load):
+
</ins><span class="cx"> 2015-08-12  Yusuke Suzuki  &lt;utatane.tea@gmail.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [ES6] Add ES6 Modules preparsing phase to collect the dependencies
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (188356 => 188357)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp        2015-08-12 20:51:57 UTC (rev 188356)
+++ trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp        2015-08-12 21:01:15 UTC (rev 188357)
</span><span class="lines">@@ -2581,6 +2581,15 @@
</span><span class="cx">     if (!variant.conditionSet().isEmpty())
</span><span class="cx">         loadedValue = load(loadPrediction, variant.conditionSet(), loadOp);
</span><span class="cx">     else {
</span><ins>+        if (needStructureCheck &amp;&amp; base-&gt;hasConstant()) {
+            // We did emit a structure check. That means that we have an opportunity to do constant folding
+            // here, since we didn't do it above.
+            JSValue constant = m_graph.tryGetConstantProperty(
+                base-&gt;asJSValue(), variant.structureSet(), variant.offset());
+            if (constant)
+                return weakJSConstant(constant);
+        }
+        
</ins><span class="cx">         loadedValue = handleGetByOffset(
</span><span class="cx">             loadPrediction, base, identifierNumber, variant.offset(), loadOp);
</span><span class="cx">     }
</span></span></pre>
</div>
</div>

</body>
</html>