[DFG] Convert ValueAdd(Int32, String) => MakeRope(ToString(Int32), String)
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Apr 2017 18:06:37 +0000 (18:06 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Apr 2017 18:06:37 +0000 (18:06 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215472 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/microbenchmarks/number-to-string-with-add-empty.js [new file with mode: 0644]
JSTests/microbenchmarks/number-to-string-with-add-in-loop.js [new file with mode: 0644]
JSTests/microbenchmarks/number-to-string-with-add.js [new file with mode: 0644]
JSTests/stress/number-to-string-with-add.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp

index a65310d..b7fdbe9 100644 (file)
@@ -1,5 +1,27 @@
 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>
+
         [DFG] Drop unknown use of CheckCell's child2 to work ObjectAllocationSinking for Array iterator object
         https://bugs.webkit.org/show_bug.cgi?id=170940
 
diff --git a/JSTests/microbenchmarks/number-to-string-with-add-empty.js b/JSTests/microbenchmarks/number-to-string-with-add-empty.js
new file mode 100644 (file)
index 0000000..0479768
--- /dev/null
@@ -0,0 +1,16 @@
+function toStringLeft(num)
+{
+    return num + '';
+}
+noInline(toStringLeft);
+
+function toStringRight(num)
+{
+    return '' + num;
+}
+noInline(toStringRight);
+
+for (var i = 0; i < 1e5; ++i) {
+    toStringLeft(i);
+    toStringRight(i);
+}
diff --git a/JSTests/microbenchmarks/number-to-string-with-add-in-loop.js b/JSTests/microbenchmarks/number-to-string-with-add-in-loop.js
new file mode 100644 (file)
index 0000000..9bc6543
--- /dev/null
@@ -0,0 +1,16 @@
+function toStringLeft(num)
+{
+    return num + 'Cocoa';
+}
+
+function toStringRight(num)
+{
+    return 'Cocoa' + num;
+}
+(function () {
+    // Hoisting.
+    for (var i = 0; i < 1e6; ++i) {
+        toStringLeft(i);
+        toStringRight(i);
+    }
+}());
diff --git a/JSTests/microbenchmarks/number-to-string-with-add.js b/JSTests/microbenchmarks/number-to-string-with-add.js
new file mode 100644 (file)
index 0000000..d286822
--- /dev/null
@@ -0,0 +1,16 @@
+function toStringLeft(num)
+{
+    return num + 'Cocoa';
+}
+noInline(toStringLeft);
+
+function toStringRight(num)
+{
+    return 'Cocoa' + num;
+}
+noInline(toStringRight);
+
+for (var i = 0; i < 1e5; ++i) {
+    toStringLeft(i);
+    toStringRight(i);
+}
diff --git a/JSTests/stress/number-to-string-with-add.js b/JSTests/stress/number-to-string-with-add.js
new file mode 100644 (file)
index 0000000..2722864
--- /dev/null
@@ -0,0 +1,40 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error(`bad value: expected:(${expected}),actual:(${actual})`);
+}
+
+function toStringLeft(num)
+{
+    return num + 'Cocoa';
+}
+noInline(toStringLeft);
+
+function toStringRight(num)
+{
+    return 'Cocoa' + num;
+}
+noInline(toStringRight);
+
+function toStringLeftEmpty(num)
+{
+    return num + '';
+}
+noInline(toStringLeftEmpty);
+
+function toStringRightEmpty(num)
+{
+    return '' + num;
+}
+noInline(toStringRightEmpty);
+
+for (var i = 0; i < 1e4; ++i) {
+    shouldBe(toStringLeft(2), `2Cocoa`);
+    shouldBe(toStringRight(2), `Cocoa2`);
+    shouldBe(toStringLeftEmpty(2), `2`);
+    shouldBe(toStringRightEmpty(2), `2`);
+    shouldBe(toStringLeft(42), `42Cocoa`);
+    shouldBe(toStringRight(42), `Cocoa42`);
+    shouldBe(toStringLeftEmpty(42), `42`);
+    shouldBe(toStringRightEmpty(42), `42`);
+}
index 3d5a9e9..14cff10 100644 (file)
@@ -1,3 +1,47 @@
+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>
 
         REGRESSION(215272): microbenchmark/seal-and-do-work and microbenchmark/freeze-and-do-work are 27x slower
index 823bda8..fb5e8b1 100644 (file)
@@ -164,8 +164,27 @@ private:
             if (attemptToMakeFastStringAdd(node))
                 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;
         }
@@ -1927,9 +1946,20 @@ private:
     template<UseKind useKind>
     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);
     }
     
     template<UseKind useKind>
index 7c9bc45..8753fe1 100644 (file)
@@ -191,7 +191,7 @@ private:
                         changed |= mergePrediction(SpecInt52Only);
                     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);
                 } else {