Changeset 215472 in webkit

Timestamp:

Apr 18, 2017 11:06:37 AM (7 years ago)

Author:

Yusuke Suzuki

Message:

[DFG] Convert ValueAdd(Int32, String) => MakeRope(ToString(Int32), String)
​https://bugs.webkit.org/show_bug.cgi?id=170943

Reviewed by Geoffrey Garen.

JSTests:

• microbenchmarks/number-to-string-with-add-empty.js: Added.

(toStringLeft):
(toStringRight):

• microbenchmarks/number-to-string-with-add-in-loop.js: Added.

(toStringLeft):
(toStringRight):

• microbenchmarks/number-to-string-with-add.js: Added.

(toStringLeft):
(toStringRight):

• stress/number-to-string-with-add.js: Added.

(shouldBe):
(toStringRight):
(toStringLeftEmpty):
(toStringRightEmpty):

Source/JavaScriptCore:

This patch converts ValueAdd(Int32, String) to MakeRope(ToString(Int32), String).
This has 2 great features.

1. MakeRope(ToString(Int32), String) is less clobbering.

While ValueAdd ends up calling functions, VM knows much about MakeRope(ToString(Int32), String)
and VM knows it is less clobbering. It encourages LICM and other operations that is conservatively
executed because of ValueAdd's clobbering.

2. Simply, MakeRope(ToString(Int32), String) is faster than ValueAdd.

While ValueAdd ends up calling a generic function, our ToString(Int32) calls well-optimized toString
operation. And later, MakeRope can fall into the fast path that just takes a string from a free list.
It is simply faster than ValueAdd.

We ensure that this patch shows performance improvement in attached benchmarks.

baseline patched

number-to-string-with-add-empty 16.2763+-3.3930 ^{10.3142+-1.0967} definitely 1.5780x faster
number-to-string-with-add-in-loop 168.7621+-10.9738 ^{15.5307+-3.3179} definitely 10.8664x faster
number-to-string-with-add 18.8557+-4.8292 11.6901+-2.5650 might be 1.6130x faster

In SixSpeed,

baseline patched

template_string_tag.es5 200.1027+-20.6871 ^{25.7925+-11.4052} definitely 7.7582x faster
template_string_tag.es6 331.3913+-12.1750 ^{286.6958+-26.0441} definitely 1.1559x faster
for-of-array.es5 412.4344+-23.2517 ^{272.8707+-47.2118} definitely 1.5115x faster
for-of-array.es6 504.0082+-65.5045 ^{300.3277+-12.8193} definitely 1.6782x faster

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::createToString):

• dfg/DFGPredictionPropagationPhase.cpp:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/microbenchmarks/number-to-string-with-add-empty.js (added)
• JSTests/microbenchmarks/number-to-string-with-add-in-loop.js (added)
• JSTests/microbenchmarks/number-to-string-with-add.js (added)
• JSTests/stress/number-to-string-with-add.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (modified) (2 diffs)
• Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2017-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG] Convert ValueAdd(Int32, String) => MakeRope(ToString(Int32), String)
+        https://bugs.webkit.org/show_bug.cgi?id=170943
+
+        Reviewed by Geoffrey Garen.
+
+        * microbenchmarks/number-to-string-with-add-empty.js: Added.
+        (toStringLeft):
+        (toStringRight):
+        * microbenchmarks/number-to-string-with-add-in-loop.js: Added.
+        (toStringLeft):
+        (toStringRight):
+        * microbenchmarks/number-to-string-with-add.js: Added.
+        (toStringLeft):
+        (toStringRight):
+        * stress/number-to-string-with-add.js: Added.
+        (shouldBe):
+        (toStringRight):
+        (toStringLeftEmpty):
+        (toStringRightEmpty):
+
 2017-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG] Convert ValueAdd(Int32, String) => MakeRope(ToString(Int32), String)
+        https://bugs.webkit.org/show_bug.cgi?id=170943
+
+        Reviewed by Geoffrey Garen.
+
+        This patch converts ValueAdd(Int32, String) to MakeRope(ToString(Int32), String).
+        This has 2 great features.
+
+        1. MakeRope(ToString(Int32), String) is less clobbering.
+
+        While ValueAdd ends up calling functions, VM knows much about MakeRope(ToString(Int32), String)
+        and VM knows it is less clobbering. It encourages LICM and other operations that is conservatively
+        executed because of ValueAdd's clobbering.
+
+        2. Simply, MakeRope(ToString(Int32), String) is faster than ValueAdd.
+
+        While ValueAdd ends up calling a generic function, our ToString(Int32) calls well-optimized toString
+        operation. And later, MakeRope can fall into the fast path that just takes a string from a free list.
+        It is simply faster than ValueAdd.
+
+        We ensure that this patch shows performance improvement in attached benchmarks.
+
+                                                        baseline                  patched
+
+            number-to-string-with-add-empty         16.2763+-3.3930     ^     10.3142+-1.0967        ^ definitely 1.5780x faster
+            number-to-string-with-add-in-loop      168.7621+-10.9738    ^     15.5307+-3.3179        ^ definitely 10.8664x faster
+            number-to-string-with-add               18.8557+-4.8292           11.6901+-2.5650          might be 1.6130x faster
+
+        In SixSpeed,
+
+                                               baseline                  patched
+
+            template_string_tag.es5       200.1027+-20.6871    ^     25.7925+-11.4052       ^ definitely 7.7582x faster
+            template_string_tag.es6       331.3913+-12.1750    ^    286.6958+-26.0441       ^ definitely 1.1559x faster
+            for-of-array.es5              412.4344+-23.2517    ^    272.8707+-47.2118       ^ definitely 1.5115x faster
+            for-of-array.es6              504.0082+-65.5045    ^    300.3277+-12.8193       ^ definitely 1.6782x faster
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        (JSC::DFG::FixupPhase::createToString):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+
 2017-04-18  Michael Saboff  <msaboff@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -165 +165 @@
                 break;
 
-            fixEdge<UntypedUse>(node->child1());
-            fixEdge<UntypedUse>(node->child2());
+            Edge& child1 = node->child1();
+            Edge& child2 = node->child2();
+            if (child1->shouldSpeculateString() || child2->shouldSpeculateString()) {
+                if (child1->shouldSpeculateInt32() || child2->shouldSpeculateInt32()) {
+                    auto convertString = [&](Node* node, Edge& edge) {
+                        if (edge->shouldSpeculateInt32())
+                            convertStringAddUse<Int32Use>(node, edge);
+                        else {
+                            ASSERT(edge->shouldSpeculateString());
+                            convertStringAddUse<StringUse>(node, edge);
+                        }
+                    };
+                    convertString(node, child1);
+                    convertString(node, child2);
+                    convertToMakeRope(node);
+                    break;
+                }
+            }
+
+            fixEdge<UntypedUse>(child1);
+            fixEdge<UntypedUse>(child2);
             node->setResult(NodeResultJS);
             break;
@@ -1928 +1947 @@
     void createToString(Node* node, Edge& edge)
     {
-        edge.setNode(m_insertionSet.insertNode(
+        Node* toString = m_insertionSet.insertNode(
             m_indexInBlock, SpecString, ToString, node->origin,
-            Edge(edge.node(), useKind)));
+            Edge(edge.node(), useKind));
+        switch (useKind) {
+        case Int32Use:
+        case Int52RepUse:
+        case DoubleRepUse:
+        case NotCellUse:
+            toString->clearFlags(NodeMustGenerate);
+            break;
+        default:
+            break;
+        }
+        edge.setNode(toString);
     }
     

trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp

@@ -192 +192 @@
                     else
                         changed |= mergePrediction(speculatedDoubleTypeForPredictions(left, right));
-                } else if (isStringOrStringObjectSpeculation(left) && isStringOrStringObjectSpeculation(right)) {
+                } else if (isStringOrStringObjectSpeculation(left) || isStringOrStringObjectSpeculation(right)) {
                     // left or right is definitely something other than a number.
                     changed |= mergePrediction(SpecString);