<!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>[214296] trunk</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/214296">214296</a></dd>
<dt>Author</dt> <dd>utatane.tea@gmail.com</dd>
<dt>Date</dt> <dd>2017-03-22 23:01:39 -0700 (Wed, 22 Mar 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>[JSC][DFG] Propagate AnyIntAsDouble information carefully to utilize it in fixup
https://bugs.webkit.org/show_bug.cgi?id=169914
Reviewed by Saam Barati.
JSTests:
* stress/any-int-as-double-add.js: Added.
(shouldBe):
(test):
* stress/to-this-numbers.js: Added.
(shouldBe):
(Number.prototype.toThis):
Source/JavaScriptCore:
In DFG prediction propagation phase, we pollute the prediction of GetByVal for Array::Double
as SpecDoubleReal even if the heap prediction says the proper prediction is SpecAnyIntAsDouble.
Thus, the following nodes just see the result of GetByVal(Array::Double) as double value,
and select suboptimal edge filters in fixup phase. For example, if the result of GetByVal is
SpecAnyIntAsDouble, we can see the node like ArithAdd(SpecAnyIntAsDouble, Int52) and we should
have a chance to make it ArithAdd(Check:Int52, Int52) instead of ArithAdd(Double, Double).
This patch propagates SpecAnyIntAsDouble in GetByVal(Array::Double) properly. And ValueAdd,
ArithAdd and ArithSub select AnyInt edge filters for SpecAnyIntAsDouble values. It finally
produces a Int52 specialized DFG node. And subsequent nodes using the produced one also
become Int52 specialized.
One considerable problem is that the heap prediction misses the non any int doubles. In that case,
if Int52 edge filter is used, BadType exit will occur. It updates the prediction of the value profile
of GetByVal. So, in the next time, GetByVal(Array::Double) produces more conservative predictions
and avoids exit-and-recompile loop correctly.
This change is very sensitive to the correct AI and appropriate predictions. Thus, this patch finds
and fixes some related issues. One is incorrect prediction of ToThis and another is incorrect
AI logic for Int52Rep.
This change dramatically improves kraken benchmarks' crypto-pbkdf2 and crypto-sha256-iterative
by 42.0% and 30.7%, respectively.
baseline patched
Kraken:
ai-astar 158.851+-4.132 ? 159.433+-5.176 ?
audio-beat-detection 53.193+-1.621 ? 53.391+-2.072 ?
audio-dft 103.589+-2.277 ? 104.902+-1.924 ? might be 1.0127x slower
audio-fft 40.491+-1.102 39.854+-0.755 might be 1.0160x faster
audio-oscillator 68.504+-1.721 ? 68.957+-1.725 ?
imaging-darkroom 118.367+-2.171 ? 119.581+-2.310 ? might be 1.0103x slower
imaging-desaturate 71.443+-1.461 ? 72.398+-1.918 ? might be 1.0134x slower
imaging-gaussian-blur 110.648+-4.035 109.184+-3.373 might be 1.0134x faster
json-parse-financial 60.363+-1.628 ? 61.936+-1.585 ? might be 1.0261x slower
json-stringify-tinderbox 37.903+-0.869 ? 39.559+-1.607 ? might be 1.0437x slower
stanford-crypto-aes 56.313+-1.512 ? 56.675+-1.715 ?
stanford-crypto-ccm 51.564+-1.900 ? 53.456+-2.548 ? might be 1.0367x slower
stanford-crypto-pbkdf2 129.546+-2.738 ^ 91.214+-2.027 ^ definitely 1.4202x faster
stanford-crypto-sha256-iterative 43.515+-0.730 ^ 33.292+-0.653 ^ definitely 1.3071x faster
<arithmetic> 78.878+-0.528 ^ 75.988+-0.621 ^ definitely 1.0380x faster
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::addShouldSpeculateAnyInt):
* dfg/DFGPredictionPropagationPhase.cpp:
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileArithNegate):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJSTestsChangeLog">trunk/JSTests/ChangeLog</a></li>
<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="#trunkSourceJavaScriptCoredfgDFGGraphh">trunk/Source/JavaScriptCore/dfg/DFGGraph.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGPredictionPropagationPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkJSTestsstressanyintasdoubleaddjs">trunk/JSTests/stress/any-int-as-double-add.js</a></li>
<li><a href="#trunkJSTestsstresstothisnumbersjs">trunk/JSTests/stress/to-this-numbers.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/ChangeLog (214295 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/ChangeLog 2017-03-23 04:51:47 UTC (rev 214295)
+++ trunk/JSTests/ChangeLog 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2017-03-22 Yusuke Suzuki <utatane.tea@gmail.com>
+
+ [JSC][DFG] Propagate AnyIntAsDouble information carefully to utilize it in fixup
+ https://bugs.webkit.org/show_bug.cgi?id=169914
+
+ Reviewed by Saam Barati.
+
+ * stress/any-int-as-double-add.js: Added.
+ (shouldBe):
+ (test):
+ * stress/to-this-numbers.js: Added.
+ (shouldBe):
+ (Number.prototype.toThis):
+
</ins><span class="cx"> 2017-03-22 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> Add support for Error.stackTraceLimit.
</span></span></pre></div>
<a id="trunkJSTestsstressanyintasdoubleaddjs"></a>
<div class="addfile"><h4>Added: trunk/JSTests/stress/any-int-as-double-add.js (0 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/any-int-as-double-add.js (rev 0)
+++ trunk/JSTests/stress/any-int-as-double-add.js 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -0,0 +1,43 @@
</span><ins>+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var array = [];
+
+for (var i = 0; i < 100; ++i)
+ array.push(1024 * 1024 * 1024 * 1024 + i);
+for (var i = 0; i < 100; ++i)
+ array.push(-(1024 * 1024 * 1024 * 1024 + i));
+
+array.push(2251799813685248);
+array.push(0.5);
+
+function test(array, index, value)
+{
+ return array[index] + fiatInt52(value);
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i) {
+ for (var index = 0; index < 100; ++index)
+ shouldBe(test(array, index, 20), 1024 * 1024 * 1024 * 1024 + index + 20);
+ for (var index = 0; index < 100; ++index)
+ shouldBe(test(array, index + 100, 20), -(1024 * 1024 * 1024 * 1024 + index) + 20);
+}
+
+// Int52Overflow.
+shouldBe(test(array, 200, 200), 2251799813685448);
+
+// Not AnyIntAsDouble, Int52Overflow.
+shouldBe(test(array, 201, 200), 200.5);
+
+// Recompile the code as ArithAdd(Double, Double).
+for (var i = 0; i < 1e4; ++i)
+ shouldBe(test(array, 201, 200), 200.5);
+
+shouldBe(test(array, 200, 200), 2251799813685448);
+shouldBe(test(array, 201, 200), 200.5);
+
+
</ins></span></pre></div>
<a id="trunkJSTestsstresstothisnumbersjs"></a>
<div class="addfile"><h4>Added: trunk/JSTests/stress/to-this-numbers.js (0 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/to-this-numbers.js (rev 0)
+++ trunk/JSTests/stress/to-this-numbers.js 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+Number.prototype.toThis = function toThis()
+{
+ 'use strict';
+ return this;
+};
+noInline(Number.prototype.toThis);
+
+for (var i = 0; i < 1e4; ++i) {
+ shouldBe((0.1).toThis(), 0.1);
+ shouldBe((42).toThis(), 42);
+ shouldBe((1024 * 1024 * 1024 * 1024).toThis(), (1024 * 1024 * 1024 * 1024));
+}
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (214295 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2017-03-23 04:51:47 UTC (rev 214295)
+++ trunk/Source/JavaScriptCore/ChangeLog 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -1,3 +1,61 @@
</span><ins>+2017-03-22 Yusuke Suzuki <utatane.tea@gmail.com>
+
+ [JSC][DFG] Propagate AnyIntAsDouble information carefully to utilize it in fixup
+ https://bugs.webkit.org/show_bug.cgi?id=169914
+
+ Reviewed by Saam Barati.
+
+ In DFG prediction propagation phase, we pollute the prediction of GetByVal for Array::Double
+ as SpecDoubleReal even if the heap prediction says the proper prediction is SpecAnyIntAsDouble.
+ Thus, the following nodes just see the result of GetByVal(Array::Double) as double value,
+ and select suboptimal edge filters in fixup phase. For example, if the result of GetByVal is
+ SpecAnyIntAsDouble, we can see the node like ArithAdd(SpecAnyIntAsDouble, Int52) and we should
+ have a chance to make it ArithAdd(Check:Int52, Int52) instead of ArithAdd(Double, Double).
+
+ This patch propagates SpecAnyIntAsDouble in GetByVal(Array::Double) properly. And ValueAdd,
+ ArithAdd and ArithSub select AnyInt edge filters for SpecAnyIntAsDouble values. It finally
+ produces a Int52 specialized DFG node. And subsequent nodes using the produced one also
+ become Int52 specialized.
+
+ One considerable problem is that the heap prediction misses the non any int doubles. In that case,
+ if Int52 edge filter is used, BadType exit will occur. It updates the prediction of the value profile
+ of GetByVal. So, in the next time, GetByVal(Array::Double) produces more conservative predictions
+ and avoids exit-and-recompile loop correctly.
+
+ This change is very sensitive to the correct AI and appropriate predictions. Thus, this patch finds
+ and fixes some related issues. One is incorrect prediction of ToThis and another is incorrect
+ AI logic for Int52Rep.
+
+ This change dramatically improves kraken benchmarks' crypto-pbkdf2 and crypto-sha256-iterative
+ by 42.0% and 30.7%, respectively.
+
+ baseline patched
+ Kraken:
+ ai-astar 158.851+-4.132 ? 159.433+-5.176 ?
+ audio-beat-detection 53.193+-1.621 ? 53.391+-2.072 ?
+ audio-dft 103.589+-2.277 ? 104.902+-1.924 ? might be 1.0127x slower
+ audio-fft 40.491+-1.102 39.854+-0.755 might be 1.0160x faster
+ audio-oscillator 68.504+-1.721 ? 68.957+-1.725 ?
+ imaging-darkroom 118.367+-2.171 ? 119.581+-2.310 ? might be 1.0103x slower
+ imaging-desaturate 71.443+-1.461 ? 72.398+-1.918 ? might be 1.0134x slower
+ imaging-gaussian-blur 110.648+-4.035 109.184+-3.373 might be 1.0134x faster
+ json-parse-financial 60.363+-1.628 ? 61.936+-1.585 ? might be 1.0261x slower
+ json-stringify-tinderbox 37.903+-0.869 ? 39.559+-1.607 ? might be 1.0437x slower
+ stanford-crypto-aes 56.313+-1.512 ? 56.675+-1.715 ?
+ stanford-crypto-ccm 51.564+-1.900 ? 53.456+-2.548 ? might be 1.0367x slower
+ stanford-crypto-pbkdf2 129.546+-2.738 ^ 91.214+-2.027 ^ definitely 1.4202x faster
+ stanford-crypto-sha256-iterative 43.515+-0.730 ^ 33.292+-0.653 ^ definitely 1.3071x faster
+
+ <arithmetic> 78.878+-0.528 ^ 75.988+-0.621 ^ definitely 1.0380x faster
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::addShouldSpeculateAnyInt):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileArithNegate):
+
</ins><span class="cx"> 2017-03-22 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> Add support for Error.stackTraceLimit.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGAbstractInterpreterInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h (214295 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h 2017-03-23 04:51:47 UTC (rev 214295)
+++ trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -475,7 +475,7 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- forNode(node).setType(SpecInt32Only);
</del><ins>+ forNode(node).setType(SpecAnyInt);
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.h (214295 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.h 2017-03-23 04:51:47 UTC (rev 214295)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.h 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -293,8 +293,13 @@
</span><span class="cx"> Node* left = add->child1().node();
</span><span class="cx"> Node* right = add->child2().node();
</span><span class="cx">
</span><del>- bool speculation = Node::shouldSpeculateAnyInt(left, right);
- return speculation && !hasExitSite(add, Int52Overflow);
</del><ins>+ auto isAnyIntSpeculationForAdd = [](SpeculatedType value) {
+ return !!value && (value & (SpecAnyInt | SpecAnyIntAsDouble)) == value;
+ };
+
+ return isAnyIntSpeculationForAdd(left->prediction())
+ && isAnyIntSpeculationForAdd(right->prediction())
+ && !hasExitSite(add, Int52Overflow);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool binaryArithShouldSpeculateInt32(Node* node, PredictionPass pass)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGPredictionPropagationPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp (214295 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp 2017-03-23 04:51:47 UTC (rev 214295)
+++ trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -354,8 +354,10 @@
</span><span class="cx"> case Array::Double:
</span><span class="cx"> if (arrayMode.isOutOfBounds())
</span><span class="cx"> changed |= mergePrediction(node->getHeapPrediction() | SpecDoubleReal);
</span><ins>+ else if (node->getHeapPrediction() & SpecNonIntAsDouble)
+ changed |= mergePrediction(SpecDoubleReal);
</ins><span class="cx"> else
</span><del>- changed |= mergePrediction(SpecDoubleReal);
</del><ins>+ changed |= mergePrediction(SpecAnyIntAsDouble);
</ins><span class="cx"> break;
</span><span class="cx"> case Array::Float32Array:
</span><span class="cx"> case Array::Float64Array:
</span><span class="lines">@@ -403,7 +405,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (node->child1()->shouldSpeculateNumber()) {
</span><del>- changed |= mergePrediction(SpecAnyInt);
</del><ins>+ changed |= mergePrediction(SpecBytecodeNumber);
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (214295 => 214296)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2017-03-23 04:51:47 UTC (rev 214295)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2017-03-23 06:01:39 UTC (rev 214296)
</span><span class="lines">@@ -2564,7 +2564,8 @@
</span><span class="cx"> LValue value = lowInt52(m_node->child1());
</span><span class="cx"> CheckValue* result = m_out.speculateSub(m_out.int64Zero, value);
</span><span class="cx"> blessSpeculation(result, Int52Overflow, noValue(), nullptr, m_origin);
</span><del>- speculate(NegativeZero, noValue(), 0, m_out.isZero64(result));
</del><ins>+ if (shouldCheckNegativeZero(m_node->arithMode()))
+ speculate(NegativeZero, noValue(), 0, m_out.isZero64(result));
</ins><span class="cx"> setInt52(result);
</span><span class="cx"> break;
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>