[DFG][FTL] Efficiently execute number#toString()

https://bugs.webkit.org/show_bug.cgi?id=170007 Reviewed by Keith Miller. JSTests: * microbenchmarks/number-to-string-strength-reduction.js: Added. (test): * microbenchmarks/number-to-string-with-radix-10.js: Added. (test): * microbenchmarks/number-to-string-with-radix-cse.js: Added. (test): * microbenchmarks/number-to-string-with-radix.js: Added. (test): * stress/number-to-string-strength-reduction.js: Added. (shouldBe): (test): * stress/number-to-string-with-radix-10.js: Added. (shouldBe): (test): * stress/number-to-string-with-radix-cse.js: Added. (shouldBe): (test): * stress/number-to-string-with-radix-invalid.js: Added. (shouldThrow): * stress/number-to-string-with-radix-watchpoint.js: Added. (shouldBe): (test): (i.i.1e3.Number.prototype.toString): * stress/number-to-string-with-radix.js: Added. (shouldBe): (test): Source/JavaScriptCore: In JS, the natural way to convert number to string with radix is `number.toString(radix)`. However, our IC only cares about cells. If the base value is a number, it always goes to the slow path. While extending our IC for number and boolean, the most meaningful use of this IC is calling `number.toString(radix)`. So, in this patch, we first add a fast path for this in DFG by using watchpoint. We set up a watchpoint for Number.prototype.toString. And if this watchpoint is kept alive and GetById(base, "toString")'s base should be speculated as Number, we emit Number related Checks and convert GetById to Number.prototype.toString constant. It removes costly GetById slow path, and makes it non-clobbering node (JSConstant). In addition, we add NumberToStringWithValidRadixConstant node. We have NumberToStringWithRadix node, but it may throw an error if the valid value is incorrect (for example, number.toString(2000)). So its clobbering rule is conservatively use read(World)/write(Heap). But in reality, `number.toString` is mostly called with the constant radix, and we can easily figure out this radix is valid (2 <= radix && radix < 32). We add a rule to the constant folding phase to convert NumberToStringWithRadix to NumberToStringWithValidRadixConstant. It ensures that it has valid constant radix. And we relax our clobbering rule for NumberToStringWithValidRadixConstant. Added microbenchmarks show performance improvement. baseline patched number-to-string-with-radix-cse 43.8312+-1.3017 ^ 7.4930+-0.5105 ^ definitely 5.8496x faster number-to-string-with-radix-10 7.2775+-0.5225 ^ 2.1906+-0.1864 ^ definitely 3.3222x faster number-to-string-with-radix 39.7378+-1.4921 ^ 16.6137+-0.7776 ^ definitely 2.3919x faster number-to-string-strength-reduction 94.9667+-2.7157 ^ 9.3060+-0.7202 ^ definitely 10.2049x faster * dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): * dfg/DFGClobberize.h: (JSC::DFG::clobberize): * dfg/DFGConstantFoldingPhase.cpp: (JSC::DFG::ConstantFoldingPhase::foldConstants): * dfg/DFGDoesGC.cpp: (JSC::DFG::doesGC): * dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): * dfg/DFGGraph.h: (JSC::DFG::Graph::isWatchingGlobalObjectWatchpoint): (JSC::DFG::Graph::isWatchingArrayIteratorProtocolWatchpoint): (JSC::DFG::Graph::isWatchingNumberToStringWatchpoint): * dfg/DFGNode.h: (JSC::DFG::Node::convertToNumberToStringWithValidRadixConstant): (JSC::DFG::Node::hasValidRadixConstant): (JSC::DFG::Node::validRadixConstant): * dfg/DFGNodeType.h: * dfg/DFGPredictionPropagationPhase.cpp: * dfg/DFGSafeToExecute.h: (JSC::DFG::safeToExecute): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructor): (JSC::DFG::SpeculativeJIT::compileNumberToStringWithValidRadixConstant): (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOnNumber): Deleted. * dfg/DFGSpeculativeJIT.h: * dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compile): * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): * dfg/DFGStrengthReductionPhase.cpp: (JSC::DFG::StrengthReductionPhase::handleNode): * ftl/FTLCapabilities.cpp: (JSC::FTL::canCompile): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileNode): (JSC::FTL::DFG::LowerDFGToB3::compileNumberToStringWithValidRadixConstant): * runtime/JSGlobalObject.cpp: (JSC::JSGlobalObject::JSGlobalObject): (JSC::JSGlobalObject::init): (JSC::JSGlobalObject::visitChildren): * runtime/JSGlobalObject.h: (JSC::JSGlobalObject::numberToStringWatchpoint): (JSC::JSGlobalObject::numberProtoToStringFunction const): * runtime/NumberPrototype.cpp: (JSC::NumberPrototype::finishCreation): (JSC::toStringWithRadixInternal): (JSC::toStringWithRadix): (JSC::int32ToStringInternal): (JSC::numberToStringInternal): * runtime/NumberPrototype.h: Canonical link: https://commits.webkit.org/192972@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221601 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Sep 5, 2017 · fb8d73d · fb8d73d
1 parent 297e758
commit fb8d73d
Show file tree

Hide file tree

Showing 33 changed files with 482 additions and 17 deletions.
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
@@ -1,3 +1,37 @@
+2017-09-03  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Efficiently execute number#toString()
+        https://bugs.webkit.org/show_bug.cgi?id=170007
+
+        Reviewed by Keith Miller.
+
+        * microbenchmarks/number-to-string-strength-reduction.js: Added.
+        (test):
+        * microbenchmarks/number-to-string-with-radix-10.js: Added.
+        (test):
+        * microbenchmarks/number-to-string-with-radix-cse.js: Added.
+        (test):
+        * microbenchmarks/number-to-string-with-radix.js: Added.
+        (test):
+        * stress/number-to-string-strength-reduction.js: Added.
+        (shouldBe):
+        (test):
+        * stress/number-to-string-with-radix-10.js: Added.
+        (shouldBe):
+        (test):
+        * stress/number-to-string-with-radix-cse.js: Added.
+        (shouldBe):
+        (test):
+        * stress/number-to-string-with-radix-invalid.js: Added.
+        (shouldThrow):
+        * stress/number-to-string-with-radix-watchpoint.js: Added.
+        (shouldBe):
+        (test):
+        (i.i.1e3.Number.prototype.toString):
+        * stress/number-to-string-with-radix.js: Added.
+        (shouldBe):
+        (test):
+
 2017-09-02  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [DFG] Relax arity requirement

diff --git a/JSTests/microbenchmarks/number-to-string-strength-reduction.js b/JSTests/microbenchmarks/number-to-string-strength-reduction.js
@@ -0,0 +1,9 @@
+function test()
+{
+    var target = 42;
+    return target.toString(16);
+}
+noInline(test);
+
+for (var i = 0; i < 1e6; ++i)
+    test();
diff --git a/JSTests/microbenchmarks/number-to-string-with-radix-10.js b/JSTests/microbenchmarks/number-to-string-with-radix-10.js
@@ -0,0 +1,10 @@
+function test()
+{
+    for (var i = 0; i < 10; ++i)
+        var result = i.toString(10);
+    return result;
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i)
+    test();
diff --git a/JSTests/microbenchmarks/number-to-string-with-radix-cse.js b/JSTests/microbenchmarks/number-to-string-with-radix-cse.js
@@ -0,0 +1,14 @@
+function test()
+{
+    for (var i = 0; i < 1e2; ++i) {
+        i.toString(16);
+        i.toString(16);
+        i.toString(16);
+        i.toString(16);
+        i.toString(16);
+    }
+}
+noInline(test);
+
+for (var i = 0; i < 1e3; ++i)
+    test();
diff --git a/JSTests/microbenchmarks/number-to-string-with-radix.js b/JSTests/microbenchmarks/number-to-string-with-radix.js
@@ -0,0 +1,16 @@
+function test()
+{
+    for (var i = 0; i < 10; ++i) {
+        var result = '';
+        result += i.toString(2);
+        result += i.toString(4);
+        result += i.toString(8);
+        result += i.toString(16);
+        result += i.toString(32);
+    }
+    return result;
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i)
+    test();
diff --git a/JSTests/stress/number-to-string-strength-reduction.js b/JSTests/stress/number-to-string-strength-reduction.js
@@ -0,0 +1,14 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test()
+{
+    var target = 42;
+    return target.toString(16);
+}
+noInline(test);
+
+for (var i = 0; i < 1e6; ++i)
+    shouldBe(test(), `2a`);
diff --git a/JSTests/stress/number-to-string-with-radix-10.js b/JSTests/stress/number-to-string-with-radix-10.js
@@ -0,0 +1,15 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+function test()
+{
+    for (var i = 0; i < 10; ++i)
+        shouldBe(i.toString(10), "" + i);
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i)
+    test();
diff --git a/JSTests/stress/number-to-string-with-radix-cse.js b/JSTests/stress/number-to-string-with-radix-cse.js
@@ -0,0 +1,21 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test()
+{
+    var result;
+    for (var i = 0; i < 1e2; ++i) {
+        i.toString(16);
+        i.toString(16);
+        i.toString(16);
+        i.toString(16);
+        result = i.toString(16);
+    }
+    return result;
+}
+noInline(test);
+
+for (var i = 0; i < 1e3; ++i)
+    shouldBe(test(), `63`);
diff --git a/JSTests/stress/number-to-string-with-radix-invalid.js b/JSTests/stress/number-to-string-with-radix-invalid.js
@@ -0,0 +1,24 @@
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+function test(i, radix)
+{
+    return i.toString(radix);
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i) {
+    shouldThrow(() => test(i, 42), `RangeError: toString() radix argument must be between 2 and 36`);
+}
diff --git a/JSTests/stress/number-to-string-with-radix-watchpoint.js b/JSTests/stress/number-to-string-with-radix-watchpoint.js
@@ -0,0 +1,27 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test()
+{
+    for (var i = 0; i < 10; ++i) {
+        var result = '';
+        result += i.toString(2);
+        result += i.toString(4);
+        result += i.toString(8);
+        result += i.toString(16);
+        result += i.toString(32);
+    }
+    return result;
+}
+noInline(test);
+
+var result = `1001211199`;
+for (var i = 0; i < 1e4; ++i) {
+    if (i === 1e3) {
+        Number.prototype.toString = function (radix) { return "Hello"; }
+        result = `HelloHelloHelloHelloHello`;
+    }
+    shouldBe(test(), result);
+}
diff --git a/JSTests/stress/number-to-string-with-radix.js b/JSTests/stress/number-to-string-with-radix.js
@@ -0,0 +1,21 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test()
+{
+    for (var i = 0; i < 10; ++i) {
+        var result = '';
+        result += i.toString(2);
+        result += i.toString(4);
+        result += i.toString(8);
+        result += i.toString(16);
+        result += i.toString(32);
+    }
+    return result;
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i)
+    shouldBe(test(), `1001211199`);
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,88 @@
+2017-09-03  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Efficiently execute number#toString()
+        https://bugs.webkit.org/show_bug.cgi?id=170007
+
+        Reviewed by Keith Miller.
+
+        In JS, the natural way to convert number to string with radix is `number.toString(radix)`.
+        However, our IC only cares about cells. If the base value is a number, it always goes to the slow path.
+
+        While extending our IC for number and boolean, the most meaningful use of this IC is calling `number.toString(radix)`.
+        So, in this patch, we first add a fast path for this in DFG by using watchpoint. We set up a watchpoint for
+        Number.prototype.toString. And if this watchpoint is kept alive and GetById(base, "toString")'s base should be
+        speculated as Number, we emit Number related Checks and convert GetById to Number.prototype.toString constant.
+        It removes costly GetById slow path, and makes it non-clobbering node (JSConstant).
+
+        In addition, we add NumberToStringWithValidRadixConstant node. We have NumberToStringWithRadix node, but it may
+        throw an error if the valid value is incorrect (for example, number.toString(2000)). So its clobbering rule is
+        conservatively use read(World)/write(Heap). But in reality, `number.toString` is mostly called with the constant
+        radix, and we can easily figure out this radix is valid (2 <= radix && radix < 32).
+        We add a rule to the constant folding phase to convert NumberToStringWithRadix to NumberToStringWithValidRadixConstant.
+        It ensures that it has valid constant radix. And we relax our clobbering rule for NumberToStringWithValidRadixConstant.
+
+        Added microbenchmarks show performance improvement.
+
+                                                      baseline                  patched
+
+        number-to-string-with-radix-cse           43.8312+-1.3017     ^      7.4930+-0.5105        ^ definitely 5.8496x faster
+        number-to-string-with-radix-10             7.2775+-0.5225     ^      2.1906+-0.1864        ^ definitely 3.3222x faster
+        number-to-string-with-radix               39.7378+-1.4921     ^     16.6137+-0.7776        ^ definitely 2.3919x faster
+        number-to-string-strength-reduction       94.9667+-2.7157     ^      9.3060+-0.7202        ^ definitely 10.2049x faster
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGGraph.h:
+        (JSC::DFG::Graph::isWatchingGlobalObjectWatchpoint):
+        (JSC::DFG::Graph::isWatchingArrayIteratorProtocolWatchpoint):
+        (JSC::DFG::Graph::isWatchingNumberToStringWatchpoint):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::convertToNumberToStringWithValidRadixConstant):
+        (JSC::DFG::Node::hasValidRadixConstant):
+        (JSC::DFG::Node::validRadixConstant):
+        * dfg/DFGNodeType.h:
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructor):
+        (JSC::DFG::SpeculativeJIT::compileNumberToStringWithValidRadixConstant):
+        (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOnNumber): Deleted.
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGStrengthReductionPhase.cpp:
+        (JSC::DFG::StrengthReductionPhase::handleNode):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+        (JSC::FTL::DFG::LowerDFGToB3::compileNumberToStringWithValidRadixConstant):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::JSGlobalObject):
+        (JSC::JSGlobalObject::init):
+        (JSC::JSGlobalObject::visitChildren):
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::numberToStringWatchpoint):
+        (JSC::JSGlobalObject::numberProtoToStringFunction const):
+        * runtime/NumberPrototype.cpp:
+        (JSC::NumberPrototype::finishCreation):
+        (JSC::toStringWithRadixInternal):
+        (JSC::toStringWithRadix):
+        (JSC::int32ToStringInternal):
+        (JSC::numberToStringInternal):
+        * runtime/NumberPrototype.h:
+
 2017-09-04  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [DFG] Consider increasing the number of DFG worklist threads

diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -1949,10 +1949,25 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
     }
 
-    case NumberToStringWithRadix:
+    case NumberToStringWithRadix: {
+        JSValue radixValue = forNode(node->child2()).m_value;
+        if (radixValue && radixValue.isInt32()) {
+            int32_t radix = radixValue.asInt32();
+            if (2 <= radix && radix <= 36) {
+                m_state.setFoundConstants(true);
+                forNode(node).set(m_graph, m_graph.m_vm.stringStructure.get());
+                break;
+            }
+        }
         clobberWorld(node->origin.semantic, clobberLimit);
         forNode(node).set(m_graph, m_graph.m_vm.stringStructure.get());
         break;
+    }
+
+    case NumberToStringWithValidRadixConstant: {
+        forNode(node).set(m_graph, m_graph.m_vm.stringStructure.get());
+        break;
+    }
 
     case NewStringObject: {
         ASSERT(node->structure()->classInfo() == StringObject::info());

diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
@@ -1584,9 +1584,14 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         return;
 
     case NumberToStringWithRadix:
+        // If the radix is invalid, NumberToStringWithRadix can throw an error.
         read(World);
         write(Heap);
         return;
+
+    case NumberToStringWithValidRadixConstant:
+        def(PureValue(node, node->validRadixConstant()));
+        return;
 
     case LastNodeType:
         RELEASE_ASSERT_NOT_REACHED();

diff --git a/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp b/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
@@ -645,6 +645,23 @@ class ConstantFoldingPhase : public Phase {
                 break;
             }
 
+            case NumberToStringWithRadix: {
+                JSValue radixValue = m_state.forNode(node->child2()).m_value;
+                if (radixValue && radixValue.isInt32()) {
+                    int32_t radix = radixValue.asInt32();
+                    if (2 <= radix && radix <= 36) {
+                        if (radix == 10) {
+                            node->setOpAndDefaultFlags(ToString);
+                            node->child2() = Edge();
+                        } else
+                            node->convertToNumberToStringWithValidRadixConstant(radix);
+                        changed = true;
+                        break;
+                    }
+                }
+                break;
+            }
+
             case Check: {
                 alreadyHandled = true;
                 m_interpreter.execute(indexInBlock);

diff --git a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
@@ -178,6 +178,7 @@ bool doesGC(Graph& graph, Node* node)
     case ToString:
     case CallStringConstructor:
     case NumberToStringWithRadix:
+    case NumberToStringWithValidRadixConstant:
     case In:
     case HasOwnProperty:
     case Jump: