Merge r235790 - [DFG] DFG should handle String#toString

https://bugs.webkit.org/show_bug.cgi?id=189151 Reviewed by Saam Barati. JSTests: The error message in String#toString and String#valueOf is poor, which will be handled in a separate bug[1]. [1]: https://bugs.webkit.org/show_bug.cgi?id=189357 * microbenchmarks/string-object-to-string.js: Added. (test): * microbenchmarks/string-object-value-of.js: Added. (test): * stress/string-to-string-error.js: Added. (shouldThrow): (test): * stress/string-to-string.js: Added. (shouldBe): (test1): (test2): (test3): * stress/string-value-of-error.js: Added. (shouldThrow): (test): * stress/string-value-of.js: Added. (shouldBe): (test1): (test2): (test3): Source/JavaScriptCore: We handle String#toString and String#valueOf in DFG by introducing StringValueOf node. In the fixup phase, we attempt to lower StringValueOf to the existing ToString or Identity nodes. If we fail to lower it, we have StringValueOf(UntypedUse), which may raise an error if an argument is neither String nor StringObject. The error message in String#toString and String#valueOf is poor, which will be handled in a separate bug[1]. It improves simple microbenchmarks by 53.4 - 67.6%. baseline patched string-object-to-string 21.7308+-3.3147 ^ 12.9655+-0.0527 ^ definitely 1.6760x faster string-object-value-of 20.1122+-0.0691 ^ 13.1134+-0.2482 ^ definitely 1.5337x faster [1]: https://bugs.webkit.org/show_bug.cgi?id=189357 * dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::handleIntrinsicCall): * dfg/DFGClobberize.h: (JSC::DFG::clobberize): * dfg/DFGDoesGC.cpp: (JSC::DFG::doesGC): * dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): (JSC::DFG::FixupPhase::fixupStringValueOf): * dfg/DFGNode.h: (JSC::DFG::Node::convertToToString): * dfg/DFGNodeType.h: * dfg/DFGOperations.cpp: * dfg/DFGOperations.h: * dfg/DFGPredictionPropagationPhase.cpp: * dfg/DFGSafeToExecute.h: (JSC::DFG::safeToExecute): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf): (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructor): Deleted. * dfg/DFGSpeculativeJIT.h: * dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compile): * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): * ftl/FTLCapabilities.cpp: (JSC::FTL::canCompile): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileNode): (JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructorOrStringValueOf): (JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructor): Deleted.
WebKit · Sep 19, 2018 · 245bd18 · 245bd18
1 parent b146f75
commit 245bd18
Show file tree

Hide file tree

Showing 27 changed files with 397 additions and 32 deletions.
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
@@ -1,3 +1,36 @@
+2018-09-06  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
+        [DFG] DFG should handle String#toString
+        https://bugs.webkit.org/show_bug.cgi?id=189151
+
+        Reviewed by Saam Barati.
+
+        The error message in String#toString and String#valueOf is poor, which will be
+        handled in a separate bug[1].
+
+        [1]: https://bugs.webkit.org/show_bug.cgi?id=189357
+
+        * microbenchmarks/string-object-to-string.js: Added.
+        (test):
+        * microbenchmarks/string-object-value-of.js: Added.
+        (test):
+        * stress/string-to-string-error.js: Added.
+        (shouldThrow):
+        (test):
+        * stress/string-to-string.js: Added.
+        (shouldBe):
+        (test1):
+        (test2):
+        (test3):
+        * stress/string-value-of-error.js: Added.
+        (shouldThrow):
+        (test):
+        * stress/string-value-of.js: Added.
+        (shouldBe):
+        (test1):
+        (test2):
+        (test3):
+
 2018-09-06  Michael Saboff  <msaboff@apple.com>
 
         Improper speculation type for Math.pow(NaN, 0) in Abstract Interpreter

diff --git a/JSTests/microbenchmarks/string-object-to-string.js b/JSTests/microbenchmarks/string-object-to-string.js
@@ -0,0 +1,15 @@
+const chars = 'abcdefghijklmnopqrstuvwxyz';
+var prim = '';
+for (var i = 0; i < 32768; i++) {
+    prim += chars.charAt(~~(Math.random() * 26));
+}
+const obj = new String(prim);
+
+function test(obj)
+{
+    return obj.toString();
+}
+noInline(test);
+
+for (var i = 0; i < 1e6; ++i)
+    test(obj);
diff --git a/JSTests/microbenchmarks/string-object-value-of.js b/JSTests/microbenchmarks/string-object-value-of.js
@@ -0,0 +1,15 @@
+const chars = 'abcdefghijklmnopqrstuvwxyz';
+var prim = '';
+for (var i = 0; i < 32768; i++) {
+    prim += chars.charAt(~~(Math.random() * 26));
+}
+const obj = new String(prim);
+
+function test(obj)
+{
+    return obj.valueOf();
+}
+noInline(test);
+
+for (var i = 0; i < 1e6; ++i)
+    test(obj);
diff --git a/JSTests/stress/string-to-string-error.js b/JSTests/stress/string-to-string-error.js
@@ -0,0 +1,45 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+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)}`);
+}
+
+var toString = String.prototype.toString;
+function test(string)
+{
+    return toString.call(string);
+}
+noInline(test);
+
+var object = {};
+var symbol = Symbol("Cocoa");
+for (var i = 0; i < 3e3; ++i) {
+    shouldThrow(() => test(object), `TypeError: Type error`);
+    shouldThrow(() => test(false), `TypeError: Type error`);
+    shouldThrow(() => test(true), `TypeError: Type error`);
+    shouldThrow(() => test(42), `TypeError: Type error`);
+    shouldThrow(() => test(null), `TypeError: Type error`);
+    shouldThrow(() => test(undefined), `TypeError: Type error`);
+    shouldThrow(() => test(symbol), `TypeError: Type error`);
+}
+
+var string = "Hello";
+var stringObject = new String(string);
+for (var i = 0; i < 1e2; ++i) {
+    shouldBe(test(string), string);
+    shouldBe(test(stringObject), string);
+}
diff --git a/JSTests/stress/string-to-string.js b/JSTests/stress/string-to-string.js
@@ -0,0 +1,38 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test1(string)
+{
+    return string.toString();
+}
+noInline(test1);
+
+function test2(string)
+{
+    return string.toString();
+}
+noInline(test2);
+
+function test3(string)
+{
+    return string.toString();
+}
+noInline(test3);
+
+var string = "Hello";
+var stringObject = new String(string);
+
+for (var i = 0; i < 1e6; ++i) {
+    shouldBe(test1(string), string);
+    shouldBe(test2(stringObject), string);
+    if (i & 1)
+        shouldBe(test3(string), string);
+    else
+        shouldBe(test3(stringObject), string);
+}
+
+shouldBe(test1({}), `[object Object]`);
+shouldBe(test2({}), `[object Object]`);
+shouldBe(test3({}), `[object Object]`);
diff --git a/JSTests/stress/string-value-of-error.js b/JSTests/stress/string-value-of-error.js
@@ -0,0 +1,45 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+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)}`);
+}
+
+var valueOf = String.prototype.valueOf;
+function test(string)
+{
+    return valueOf.call(string);
+}
+noInline(test);
+
+var object = {};
+var symbol = Symbol("Cocoa");
+for (var i = 0; i < 3e3; ++i) {
+    shouldThrow(() => test(object), `TypeError: Type error`);
+    shouldThrow(() => test(false), `TypeError: Type error`);
+    shouldThrow(() => test(true), `TypeError: Type error`);
+    shouldThrow(() => test(42), `TypeError: Type error`);
+    shouldThrow(() => test(null), `TypeError: Type error`);
+    shouldThrow(() => test(undefined), `TypeError: Type error`);
+    shouldThrow(() => test(symbol), `TypeError: Type error`);
+}
+
+var string = "Hello";
+var stringObject = new String(string);
+for (var i = 0; i < 1e2; ++i) {
+    shouldBe(test(string), string);
+    shouldBe(test(stringObject), string);
+}
diff --git a/JSTests/stress/string-value-of.js b/JSTests/stress/string-value-of.js
@@ -0,0 +1,39 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test1(string)
+{
+    return string.valueOf();
+}
+noInline(test1);
+
+function test2(string)
+{
+    return string.valueOf();
+}
+noInline(test2);
+
+function test3(string)
+{
+    return string.valueOf();
+}
+noInline(test3);
+
+var string = "Hello";
+var stringObject = new String(string);
+
+for (var i = 0; i < 1e6; ++i) {
+    shouldBe(test1(string), string);
+    shouldBe(test2(stringObject), string);
+    if (i & 1)
+        shouldBe(test3(string), string);
+    else
+        shouldBe(test3(stringObject), string);
+}
+
+var object = {};
+shouldBe(test1(object), object);
+shouldBe(test2(object), object);
+shouldBe(test3(object), object);
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,59 @@
+2018-09-06  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
+        [DFG] DFG should handle String#toString
+        https://bugs.webkit.org/show_bug.cgi?id=189151
+
+        Reviewed by Saam Barati.
+
+        We handle String#toString and String#valueOf in DFG by introducing StringValueOf node.
+        In the fixup phase, we attempt to lower StringValueOf to the existing ToString or Identity
+        nodes. If we fail to lower it, we have StringValueOf(UntypedUse), which may raise an error
+        if an argument is neither String nor StringObject. The error message in String#toString and
+        String#valueOf is poor, which will be handled in a separate bug[1].
+
+        It improves simple microbenchmarks by 53.4 - 67.6%.
+
+                                              baseline                  patched
+
+            string-object-to-string       21.7308+-3.3147     ^     12.9655+-0.0527        ^ definitely 1.6760x faster
+            string-object-value-of        20.1122+-0.0691     ^     13.1134+-0.2482        ^ definitely 1.5337x faster
+
+        [1]: https://bugs.webkit.org/show_bug.cgi?id=189357
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleIntrinsicCall):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        (JSC::DFG::FixupPhase::fixupStringValueOf):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::convertToToString):
+        * dfg/DFGNodeType.h:
+        * dfg/DFGOperations.cpp:
+        * dfg/DFGOperations.h:
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf):
+        (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructor): Deleted.
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+        (JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructorOrStringValueOf):
+        (JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructor): Deleted.
+
 2018-09-07  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         [JSC] Put .throwStackOverflow code after the fast path in LLInt doVMEntry

diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -1175,6 +1175,12 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
     }
 
+    case StringValueOf: {
+        clobberWorld();
+        setTypeForNode(node, SpecString);
+        break;
+    }
+
     case StringSlice: {
         setTypeForNode(node, SpecString);
         break;

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -2691,6 +2691,13 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
         return true;
     }
 
+    case StringPrototypeValueOfIntrinsic: {
+        insertChecks();
+        Node* value = get(virtualRegisterForArgument(0, registerOffset));
+        set(VirtualRegister(resultOperand), addToGraph(StringValueOf, value));
+        return true;
+    }
+
     case StringPrototypeReplaceIntrinsic: {
         if (argumentCountIncludingThis != 3)
             return false;

diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
@@ -652,6 +652,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     case NumberToStringWithRadix:
     case CreateThis:
     case InstanceOf:
+    case StringValueOf:
         read(World);
         write(Heap);
         return;

diff --git a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
@@ -363,6 +363,7 @@ bool doesGC(Graph& graph, Node* node)
     case StringReplace:
     case StringReplaceRegExp:
     case StringSlice:
+    case StringValueOf:
     case CreateRest:
     case ToLowerCase:
     case CallDOMGetter:

diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -1994,6 +1994,11 @@ class FixupPhase : public Phase {
             break;
         }
 
+        case StringValueOf: {
+            fixupStringValueOf(node);
+            break;
+        }
+
         case StringSlice: {
             fixEdge<StringUse>(node->child1());
             fixEdge<Int32Use>(node->child2());
@@ -2753,6 +2758,31 @@ class FixupPhase : public Phase {
         }
     }
 
+    void fixupStringValueOf(Node* node)
+    {
+        if (node->child1()->shouldSpeculateString()) {
+            fixEdge<StringUse>(node->child1());
+            node->convertToIdentity();
+            return;
+        }
+
+        if (node->child1()->shouldSpeculateStringObject()) {
+            fixEdge<StringObjectUse>(node->child1());
+            node->convertToToString();
+            // It does not need to look up a toString property for the StringObject case. So we can clear NodeMustGenerate.
+            node->clearFlags(NodeMustGenerate);
+            return;
+        }
+
+        if (node->child1()->shouldSpeculateStringOrStringObject()) {
+            fixEdge<StringOrStringObjectUse>(node->child1());
+            node->convertToToString();
+            // It does not need to look up a toString property for the StringObject case. So we can clear NodeMustGenerate.
+            node->clearFlags(NodeMustGenerate);
+            return;
+        }
+    }
+
     bool attemptToMakeFastStringAdd(Node* node)
     {
         bool goodToGo = true;

diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -694,7 +694,7 @@ struct Node {
 
     void convertToToString()
     {
-        ASSERT(m_op == ToPrimitive);
+        ASSERT(m_op == ToPrimitive || m_op == StringValueOf);
         m_op = ToString;
     }