[DFG][FTL] Add NewSymbol
authoryusukesuzuki@slowstart.org <yusukesuzuki@slowstart.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 02:09:28 +0000 (02:09 +0000)
committeryusukesuzuki@slowstart.org <yusukesuzuki@slowstart.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 02:09:28 +0000 (02:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192620

Reviewed by Saam Barati.

JSTests:

* microbenchmarks/symbol-creation.js: Added.
(test):
* stress/symbol-description-identity.js: Added.
(shouldBe):
(test):
* stress/symbol-identity.js: Added.
(shouldBe):
(test):
* stress/symbol-with-description-throw-error.js: Added.
(shouldBe):
(shouldThrow):
(test):
(object.toString):

Source/JavaScriptCore:

This patch introduces NewSymbol DFG node into DFG and FTL tiers. The main goal of this patch is not optimize
NewSymbol code faster. Rather than that, this patch intends to offer SpecSymbol type information into DFG's
data flow to optimize generated code in FTL backend.

We add NewSymbol DFG node, which may take an argument. If an argument is not given, NewSymbol is for `Symbol()`.
If an argument is given, ToString is emitted to this argument before passing it to NewSymbol. So NewSymbol node
itself does not perform any type checks. ToString performs effects, but NewSymbol doesn't have any side observable
effects. So we can decouple Symbol(description) call into NewSymbol(ToString(description)).

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleConstantInternalFunction):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGClobbersExitState.cpp:
(JSC::DFG::clobbersExitState):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGMayExit.cpp:
* dfg/DFGNodeType.h:
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileNewSymbol):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGStoreBarrierInsertionPhase.cpp:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileNewSymbol):

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

25 files changed:
JSTests/ChangeLog
JSTests/microbenchmarks/symbol-creation.js [new file with mode: 0644]
JSTests/stress/symbol-description-identity.js [new file with mode: 0644]
JSTests/stress/symbol-identity.js [new file with mode: 0644]
JSTests/stress/symbol-with-description-throw-error.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGClobbersExitState.cpp
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGMayExit.cpp
Source/JavaScriptCore/dfg/DFGNodeType.h
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/dfg/DFGOperations.h
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index c9dd1bc..2302345 100644 (file)
@@ -1,5 +1,26 @@
 2018-12-12  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
+        [DFG][FTL] Add NewSymbol
+        https://bugs.webkit.org/show_bug.cgi?id=192620
+
+        Reviewed by Saam Barati.
+
+        * microbenchmarks/symbol-creation.js: Added.
+        (test):
+        * stress/symbol-description-identity.js: Added.
+        (shouldBe):
+        (test):
+        * stress/symbol-identity.js: Added.
+        (shouldBe):
+        (test):
+        * stress/symbol-with-description-throw-error.js: Added.
+        (shouldBe):
+        (shouldThrow):
+        (test):
+        (object.toString):
+
+2018-12-12  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
         [BigInt] Implement DFG/FTL typeof for BigInt
         https://bugs.webkit.org/show_bug.cgi?id=192619
 
diff --git a/JSTests/microbenchmarks/symbol-creation.js b/JSTests/microbenchmarks/symbol-creation.js
new file mode 100644 (file)
index 0000000..baf260b
--- /dev/null
@@ -0,0 +1,8 @@
+function test()
+{
+    return Symbol();
+}
+noInline(test);
+
+for (var i = 0; i < 4e5; ++i)
+    test();
diff --git a/JSTests/stress/symbol-description-identity.js b/JSTests/stress/symbol-description-identity.js
new file mode 100644 (file)
index 0000000..3f0be68
--- /dev/null
@@ -0,0 +1,21 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+function test(description)
+{
+    return Symbol(description);
+}
+noInline(test);
+
+var set = new Set;
+for (var i = 0; i < 1e4; ++i) {
+    var description = String(i);
+    var symbol = test(description);
+    set.add(symbol);
+    shouldBe(set.size, i + 1);
+    shouldBe(symbol.description, description);
+}
diff --git a/JSTests/stress/symbol-identity.js b/JSTests/stress/symbol-identity.js
new file mode 100644 (file)
index 0000000..14da8ba
--- /dev/null
@@ -0,0 +1,20 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+function test()
+{
+    return Symbol();
+}
+noInline(test);
+
+var set = new Set;
+for (var i = 0; i < 1e4; ++i) {
+    var symbol = test();
+    set.add(symbol);
+    shouldBe(set.size, i + 1);
+    shouldBe(symbol.description, undefined);
+}
diff --git a/JSTests/stress/symbol-with-description-throw-error.js b/JSTests/stress/symbol-with-description-throw-error.js
new file mode 100644 (file)
index 0000000..45b2f51
--- /dev/null
@@ -0,0 +1,52 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+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)}`);
+}
+noInline(shouldThrow);
+
+function test(description)
+{
+    return Symbol(description);
+}
+noInline(test);
+
+var count = 0;
+var flag = false;
+var object = {
+    toString()
+    {
+        count++;
+        if (flag) {
+            throw new Error("out");
+        }
+        return "Cocoa";
+    }
+};
+
+for (var i = 0; i < 1e4; ++i) {
+    shouldBe(test(object).description, "Cocoa");
+    shouldBe(count, i + 1);
+}
+flag = true;
+
+shouldThrow(() => {
+    shouldBe(test(object).description, "Cocoa");
+    shouldBe(count, 1e4 + 1);
+}, `Error: out`);
index f6afc77..3756656 100644 (file)
@@ -1,5 +1,54 @@
 2018-12-12  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
+        [DFG][FTL] Add NewSymbol
+        https://bugs.webkit.org/show_bug.cgi?id=192620
+
+        Reviewed by Saam Barati.
+
+        This patch introduces NewSymbol DFG node into DFG and FTL tiers. The main goal of this patch is not optimize
+        NewSymbol code faster. Rather than that, this patch intends to offer SpecSymbol type information into DFG's
+        data flow to optimize generated code in FTL backend.
+
+        We add NewSymbol DFG node, which may take an argument. If an argument is not given, NewSymbol is for `Symbol()`.
+        If an argument is given, ToString is emitted to this argument before passing it to NewSymbol. So NewSymbol node
+        itself does not perform any type checks. ToString performs effects, but NewSymbol doesn't have any side observable
+        effects. So we can decouple Symbol(description) call into NewSymbol(ToString(description)).
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleConstantInternalFunction):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGClobbersExitState.cpp:
+        (JSC::DFG::clobbersExitState):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGMayExit.cpp:
+        * dfg/DFGNodeType.h:
+        * dfg/DFGOperations.cpp:
+        * dfg/DFGOperations.h:
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileNewSymbol):
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGStoreBarrierInsertionPhase.cpp:
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+        (JSC::FTL::DFG::LowerDFGToB3::compileNewSymbol):
+
+2018-12-12  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
         [BigInt] Implement DFG/FTL typeof for BigInt
         https://bugs.webkit.org/show_bug.cgi?id=192619
 
index c8f66dd..f592dd1 100644 (file)
@@ -2424,6 +2424,11 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         setForNode(node, node->structure());
         break;
     }
+
+    case NewSymbol: {
+        setForNode(node, m_vm.symbolStructure.get());
+        break;
+    }
             
     case NewArray:
         ASSERT(node->indexingMode() == node->indexingType()); // Copy on write arrays should only be created by NewArrayBuffer.
index aa23167..edb0c64 100644 (file)
@@ -65,6 +65,7 @@
 #include "StackAlignment.h"
 #include "StringConstructor.h"
 #include "StructureStubInfo.h"
+#include "SymbolConstructor.h"
 #include "Watchdog.h"
 #include <wtf/CommaPrinter.h>
 #include <wtf/HashMap.h>
@@ -3712,6 +3713,20 @@ bool ByteCodeParser::handleConstantInternalFunction(
         return true;
     }
 
+    if (function->classInfo() == SymbolConstructor::info() && kind == CodeForCall) {
+        insertChecks();
+
+        Node* resultNode;
+
+        if (argumentCountIncludingThis <= 1)
+            resultNode = addToGraph(NewSymbol);
+        else
+            resultNode = addToGraph(NewSymbol, addToGraph(ToString, get(virtualRegisterForArgument(1, registerOffset))));
+
+        set(result, resultNode);
+        return true;
+    }
+
     // FIXME: This should handle construction as well. https://bugs.webkit.org/show_bug.cgi?id=155591
     if (function->classInfo() == ObjectConstructor::info() && kind == CodeForCall) {
         insertChecks();
index 95404f3..166e419 100644 (file)
@@ -1531,6 +1531,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
 
     case NewObject:
     case NewRegexp:
+    case NewSymbol:
     case NewStringObject:
     case PhantomNewObject:
     case MaterializeNewObject:
index facdbfb..fddd518 100644 (file)
@@ -58,6 +58,7 @@ bool clobbersExitState(Graph& graph, Node* node)
     case Arrayify:
     case NewObject:
     case NewRegexp:
+    case NewSymbol:
     case NewStringObject:
     case PhantomNewObject:
     case MaterializeNewObject:
index 64e2f5e..808932f 100644 (file)
@@ -350,6 +350,7 @@ bool doesGC(Graph& graph, Node* node)
     case NewArrayBuffer:
     case NewRegexp:
     case NewStringObject:
+    case NewSymbol:
     case MakeRope:
     case NewFunction:
     case NewGeneratorFunction:
index 5b0de12..593ddf2 100644 (file)
@@ -1274,6 +1274,12 @@ private:
             break;
         }
 
+        case NewSymbol: {
+            if (node->child1())
+                fixEdge<KnownStringUse>(node->child1());
+            break;
+        }
+
         case NewArrayWithSpread: {
             watchHavingABadTime(node);
             
index 11d90d1..5cbfae8 100644 (file)
@@ -124,6 +124,7 @@ ExitMode mayExitImpl(Graph& graph, Node* node, StateType& state)
     case NewAsyncFunction:
     case NewAsyncGeneratorFunction:
     case NewStringObject:
+    case NewSymbol:
     case NewRegexp:
     case ToNumber:
     case RegExpExecNonGlobalOrSticky:
index 40f6264..3817fb2 100644 (file)
@@ -336,6 +336,8 @@ namespace JSC { namespace DFG {
     macro(NewArrayBuffer, NodeResultJS) \
     macro(NewTypedArray, NodeResultJS | NodeMustGenerate) \
     macro(NewRegexp, NodeResultJS) \
+    macro(NewSymbol, NodeResultJS) \
+    macro(NewStringObject, NodeResultJS) \
     /* Rest Parameter */\
     macro(GetRestLength, NodeResultInt32) \
     macro(CreateRest, NodeResultJS | NodeMustGenerate) \
@@ -380,7 +382,6 @@ namespace JSC { namespace DFG {
     macro(CallStringConstructor, NodeResultJS | NodeMustGenerate) \
     macro(NumberToStringWithRadix, NodeResultJS | NodeMustGenerate) \
     macro(NumberToStringWithValidRadixConstant, NodeResultJS) \
-    macro(NewStringObject, NodeResultJS) \
     macro(MakeRope, NodeResultJS) \
     macro(InByVal, NodeResultBoolean | NodeMustGenerate) \
     macro(InById, NodeResultBoolean | NodeMustGenerate) \
index be992af..81f16c5 100644 (file)
@@ -2230,6 +2230,22 @@ JSString* JIT_OPERATION operationSingleCharacterString(ExecState* exec, int32_t
     return jsSingleCharacterString(exec, static_cast<UChar>(character));
 }
 
+Symbol* JIT_OPERATION operationNewSymbol(ExecState* exec)
+{
+    VM& vm = exec->vm();
+    NativeCallFrameTracer tracer(&vm, exec);
+
+    return Symbol::create(vm);
+}
+
+Symbol* JIT_OPERATION operationNewSymbolWithDescription(ExecState* exec, JSString* description)
+{
+    VM& vm = exec->vm();
+    NativeCallFrameTracer tracer(&vm, exec);
+
+    return Symbol::create(exec, description);
+}
+
 JSCell* JIT_OPERATION operationNewStringObject(ExecState* exec, JSString* string, Structure* structure)
 {
     VM& vm = exec->vm();
index e455609..4e35634 100644 (file)
@@ -219,6 +219,8 @@ EncodedJSValue JIT_OPERATION operationParseIntStringNoRadix(ExecState*, JSString
 EncodedJSValue JIT_OPERATION operationParseIntString(ExecState*, JSString*, int32_t);
 EncodedJSValue JIT_OPERATION operationParseIntGeneric(ExecState*, EncodedJSValue, int32_t);
 
+Symbol* JIT_OPERATION operationNewSymbol(ExecState*);
+Symbol* JIT_OPERATION operationNewSymbolWithDescription(ExecState*, JSString*);
 JSCell* JIT_OPERATION operationNewStringObject(ExecState*, JSString*, Structure*);
 JSString* JIT_OPERATION operationToStringOnCell(ExecState*, JSCell*);
 JSString* JIT_OPERATION operationToString(ExecState*, EncodedJSValue);
index e8299eb..72e15fa 100644 (file)
@@ -1019,6 +1019,10 @@ private:
             setPrediction(SpecStringObject);
             break;
         }
+        case NewSymbol: {
+            setPrediction(SpecSymbol);
+            break;
+        }
             
         case CreateDirectArguments: {
             setPrediction(SpecDirectArguments);
index 07cf01f..dc751e5 100644 (file)
@@ -315,6 +315,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node, bool igno
     case NewArrayWithSpread:
     case Spread:
     case NewRegexp:
+    case NewSymbol:
     case ProfileType:
     case ProfileControlFlow:
     case CheckTypeInfoFlags:
index 465bf3f..ea659a8 100644 (file)
@@ -9647,6 +9647,32 @@ void SpeculativeJIT::compileNewStringObject(Node* node)
     cellResult(resultGPR, node);
 }
 
+void SpeculativeJIT::compileNewSymbol(Node* node)
+{
+    if (!node->child1()) {
+        flushRegisters();
+        GPRFlushedCallResult result(this);
+        GPRReg resultGPR = result.gpr();
+        callOperation(operationNewSymbol, resultGPR);
+        m_jit.exceptionCheck();
+        cellResult(resultGPR, node);
+        return;
+    }
+
+
+    ASSERT(node->child1().useKind() == KnownStringUse);
+    SpeculateCellOperand operand(this, node->child1());
+
+    GPRReg stringGPR = operand.gpr();
+
+    flushRegisters();
+    GPRFlushedCallResult result(this);
+    GPRReg resultGPR = result.gpr();
+    callOperation(operationNewSymbolWithDescription, resultGPR, stringGPR);
+    m_jit.exceptionCheck();
+    cellResult(resultGPR, node);
+}
+
 void SpeculativeJIT::compileNewTypedArrayWithSize(Node* node)
 {
     JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node->origin.semantic);
index a20e39a..8177e4a 100644 (file)
@@ -1253,6 +1253,7 @@ public:
     void compileNumberToStringWithValidRadixConstant(Node*);
     void compileNumberToStringWithValidRadixConstant(Node*, int32_t radix);
     void compileNewStringObject(Node*);
+    void compileNewSymbol(Node*);
     
     void compileNewTypedArrayWithSize(Node*);
     
index bcf7c72..0c973fc 100644 (file)
@@ -3105,6 +3105,11 @@ void SpeculativeJIT::compile(Node* node)
         compileNewStringObject(node);
         break;
     }
+
+    case NewSymbol: {
+        compileNewSymbol(node);
+        break;
+    }
         
     case NewArray: {
         compileNewArray(node);
index 0662744..25215e3 100644 (file)
@@ -3347,6 +3347,11 @@ void SpeculativeJIT::compile(Node* node)
         compileNewStringObject(node);
         break;
     }
+
+    case NewSymbol: {
+        compileNewSymbol(node);
+        break;
+    }
         
     case NewArray: {
         compileNewArray(node);
index 68b063e..6af45b0 100644 (file)
@@ -325,9 +325,10 @@ private:
             case NewArrayBuffer:
             case NewTypedArray:
             case NewRegexp:
+            case NewStringObject:
+            case NewSymbol:
             case MaterializeNewObject:
             case MaterializeCreateActivation:
-            case NewStringObject:
             case MakeRope:
             case CreateActivation:
             case CreateDirectArguments:
index 0a3569f..c91d155 100644 (file)
@@ -74,6 +74,7 @@ inline CapabilityLevel canCompile(Node* node)
     case GetButterfly:
     case NewObject:
     case NewStringObject:
+    case NewSymbol:
     case NewArray:
     case NewArrayWithSpread:
     case Spread:
index 5176fef..85f40ed 100644 (file)
@@ -869,6 +869,9 @@ private:
         case NewStringObject:
             compileNewStringObject();
             break;
+        case NewSymbol:
+            compileNewSymbol();
+            break;
         case NewArray:
             compileNewArray();
             break;
@@ -5535,7 +5538,17 @@ private:
         m_out.appendTo(continuation, lastNext);
         setJSValue(m_out.phi(pointerType(), fastResult, slowResult));
     }
-    
+
+    void compileNewSymbol()
+    {
+        if (!m_node->child1()) {
+            setJSValue(vmCall(pointerType(), m_out.operation(operationNewSymbol), m_callFrame));
+            return;
+        }
+        ASSERT(m_node->child1().useKind() == KnownStringUse);
+        setJSValue(vmCall(pointerType(), m_out.operation(operationNewSymbolWithDescription), m_callFrame, lowString(m_node->child1())));
+    }
+
     void compileNewArray()
     {
         // First speculate appropriately on all of the children. Do this unconditionally up here