- Revision
- 214789
- Author
- carlo...@webkit.org
- Date
- 2017-04-03 06:32:18 -0700 (Mon, 03 Apr 2017)
Log Message
Merge r214296 - [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/_javascript_Core:
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):
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/JSTests/ChangeLog (214788 => 214789)
--- releases/WebKitGTK/webkit-2.16/JSTests/ChangeLog 2017-04-03 13:25:44 UTC (rev 214788)
+++ releases/WebKitGTK/webkit-2.16/JSTests/ChangeLog 2017-04-03 13:32:18 UTC (rev 214789)
@@ -1,5 +1,19 @@
2017-03-22 Yusuke Suzuki <utatane....@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):
+
+2017-03-22 Yusuke Suzuki <utatane....@gmail.com>
+
[JSC] Use jsNontrivialString for Number toString operations
https://bugs.webkit.org/show_bug.cgi?id=169965
Added: releases/WebKitGTK/webkit-2.16/JSTests/stress/any-int-as-double-add.js (0 => 214789)
--- releases/WebKitGTK/webkit-2.16/JSTests/stress/any-int-as-double-add.js (rev 0)
+++ releases/WebKitGTK/webkit-2.16/JSTests/stress/any-int-as-double-add.js 2017-04-03 13:32:18 UTC (rev 214789)
@@ -0,0 +1,43 @@
+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);
+
+
Added: releases/WebKitGTK/webkit-2.16/JSTests/stress/to-this-numbers.js (0 => 214789)
--- releases/WebKitGTK/webkit-2.16/JSTests/stress/to-this-numbers.js (rev 0)
+++ releases/WebKitGTK/webkit-2.16/JSTests/stress/to-this-numbers.js 2017-04-03 13:32:18 UTC (rev 214789)
@@ -0,0 +1,19 @@
+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));
+}
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog (214788 => 214789)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog 2017-04-03 13:25:44 UTC (rev 214788)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog 2017-04-03 13:32:18 UTC (rev 214789)
@@ -1,5 +1,63 @@
2017-03-22 Yusuke Suzuki <utatane....@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):
+
+2017-03-22 Yusuke Suzuki <utatane....@gmail.com>
+
[JSC] Use jsNontrivialString for Number toString operations
https://bugs.webkit.org/show_bug.cgi?id=169965
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (214788 => 214789)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2017-04-03 13:25:44 UTC (rev 214788)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2017-04-03 13:32:18 UTC (rev 214789)
@@ -475,7 +475,7 @@
break;
}
- forNode(node).setType(SpecInt32Only);
+ forNode(node).setType(SpecAnyInt);
break;
}
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGGraph.h (214788 => 214789)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGGraph.h 2017-04-03 13:25:44 UTC (rev 214788)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGGraph.h 2017-04-03 13:32:18 UTC (rev 214789)
@@ -293,8 +293,13 @@
Node* left = add->child1().node();
Node* right = add->child2().node();
- bool speculation = Node::shouldSpeculateAnyInt(left, right);
- return speculation && !hasExitSite(add, Int52Overflow);
+ auto isAnyIntSpeculationForAdd = [](SpeculatedType value) {
+ return !!value && (value & (SpecAnyInt | SpecAnyIntAsDouble)) == value;
+ };
+
+ return isAnyIntSpeculationForAdd(left->prediction())
+ && isAnyIntSpeculationForAdd(right->prediction())
+ && !hasExitSite(add, Int52Overflow);
}
bool binaryArithShouldSpeculateInt32(Node* node, PredictionPass pass)
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (214788 => 214789)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2017-04-03 13:25:44 UTC (rev 214788)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2017-04-03 13:32:18 UTC (rev 214789)
@@ -354,8 +354,10 @@
case Array::Double:
if (arrayMode.isOutOfBounds())
changed |= mergePrediction(node->getHeapPrediction() | SpecDoubleReal);
+ else if (node->getHeapPrediction() & SpecNonIntAsDouble)
+ changed |= mergePrediction(SpecDoubleReal);
else
- changed |= mergePrediction(SpecDoubleReal);
+ changed |= mergePrediction(SpecAnyIntAsDouble);
break;
case Array::Float32Array:
case Array::Float64Array:
@@ -403,7 +405,7 @@
}
if (node->child1()->shouldSpeculateNumber()) {
- changed |= mergePrediction(SpecAnyInt);
+ changed |= mergePrediction(SpecBytecodeNumber);
break;
}
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (214788 => 214789)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-04-03 13:25:44 UTC (rev 214788)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-04-03 13:32:18 UTC (rev 214789)
@@ -2562,7 +2562,8 @@
LValue value = lowInt52(m_node->child1());
CheckValue* result = m_out.speculateSub(m_out.int64Zero, value);
blessSpeculation(result, Int52Overflow, noValue(), nullptr, m_origin);
- speculate(NegativeZero, noValue(), 0, m_out.isZero64(result));
+ if (shouldCheckNegativeZero(m_node->arithMode()))
+ speculate(NegativeZero, noValue(), 0, m_out.isZero64(result));
setInt52(result);
break;
}
