[ES6] DFG and FTL should be aware of that StringConstructor behavior for symbols...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Apr 2015 19:07:12 +0000 (19:07 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Apr 2015 19:07:12 +0000 (19:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143424

Reviewed by Geoffrey Garen.

In ES6, StringConstructor behavior becomes different from ToString abstract operations in the spec. (and JSValue::toString).

ToString(symbol) throws a type error.
However, String(symbol) produces SymbolDescriptiveString(symbol).

So, in DFG and FTL phase, they should not inline StringConstructor to ToString.

Now, in the template literals patch, ToString DFG operation is planned to be used.
And current ToString behavior is aligned to the spec (and JSValue::toString) and it's better.
So intead of changing ToString behavior, this patch adds CallStringConstructor operation into DFG and FTL.
In CallStringConstructor, all behavior in DFG analysis is the same.
Only the difference from ToString is, when calling DFG operation functions, it calls
operationCallStringConstructorOnCell and operationCallStringConstructor instead of
operationToStringOnCell and operationToString.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::propagate):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleConstantInternalFunction):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
(JSC::DFG::FixupPhase::fixupToString): Deleted.
* dfg/DFGNodeType.h:
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOnCell):
(JSC::DFG::SpeculativeJIT::compileToStringOnCell): Deleted.
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGStructureRegistrationPhase.cpp:
(JSC::DFG::StructureRegistrationPhase::run):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compileToStringOrCallStringConstructor):
(JSC::FTL::LowerDFGToLLVM::compileToString): Deleted.
* runtime/StringConstructor.cpp:
(JSC::stringConstructor):
(JSC::callStringConstructor):
* runtime/StringConstructor.h:
* tests/stress/symbol-and-string-constructor.js: Added.
(performString):

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

22 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.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/DFGStructureRegistrationPhase.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
Source/JavaScriptCore/runtime/StringConstructor.cpp
Source/JavaScriptCore/runtime/StringConstructor.h
Source/JavaScriptCore/tests/stress/symbol-and-string-constructor.js [new file with mode: 0644]

index 85d5f62..8e8de5e 100644 (file)
@@ -1,5 +1,72 @@
 2015-04-06  Yusuke Suzuki  <utatane.tea@gmail.com>
 
+        [ES6] DFG and FTL should be aware of that StringConstructor behavior for symbols becomes different from ToString
+        https://bugs.webkit.org/show_bug.cgi?id=143424
+
+        Reviewed by Geoffrey Garen.
+
+        In ES6, StringConstructor behavior becomes different from ToString abstract operations in the spec. (and JSValue::toString).
+
+        ToString(symbol) throws a type error.
+        However, String(symbol) produces SymbolDescriptiveString(symbol).
+
+        So, in DFG and FTL phase, they should not inline StringConstructor to ToString.
+
+        Now, in the template literals patch, ToString DFG operation is planned to be used.
+        And current ToString behavior is aligned to the spec (and JSValue::toString) and it's better.
+        So intead of changing ToString behavior, this patch adds CallStringConstructor operation into DFG and FTL.
+        In CallStringConstructor, all behavior in DFG analysis is the same.
+        Only the difference from ToString is, when calling DFG operation functions, it calls
+        operationCallStringConstructorOnCell and operationCallStringConstructor instead of
+        operationToStringOnCell and operationToString.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGBackwardsPropagationPhase.cpp:
+        (JSC::DFG::BackwardsPropagationPhase::propagate):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleConstantInternalFunction):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        (JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
+        (JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
+        (JSC::DFG::FixupPhase::fixupToString): Deleted.
+        * dfg/DFGNodeType.h:
+        * dfg/DFGOperations.cpp:
+        * dfg/DFGOperations.h:
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::propagate):
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOnCell):
+        (JSC::DFG::SpeculativeJIT::compileToStringOnCell): Deleted.
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGStructureRegistrationPhase.cpp:
+        (JSC::DFG::StructureRegistrationPhase::run):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileNode):
+        (JSC::FTL::LowerDFGToLLVM::compileToStringOrCallStringConstructor):
+        (JSC::FTL::LowerDFGToLLVM::compileToString): Deleted.
+        * runtime/StringConstructor.cpp:
+        (JSC::stringConstructor):
+        (JSC::callStringConstructor):
+        * runtime/StringConstructor.h:
+        * tests/stress/symbol-and-string-constructor.js: Added.
+        (performString):
+
+2015-04-06  Yusuke Suzuki  <utatane.tea@gmail.com>
+
         Return Optional<uint32_t> from PropertyName::asIndex
         https://bugs.webkit.org/show_bug.cgi?id=143422
 
index 05de909..a552bcb 100644 (file)
@@ -1261,7 +1261,8 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
     }
         
-    case ToString: {
+    case ToString:
+    case CallStringConstructor: {
         switch (node->child1().useKind()) {
         case StringObjectUse:
             // This also filters that the StringObject has the primordial StringObject
index 43c3d5d..271dbc2 100644 (file)
@@ -345,7 +345,8 @@ private:
             break;
         }
             
-        case ToString: {
+        case ToString:
+        case CallStringConstructor: {
             node->child1()->mergeFlags(NodeBytecodeUsesAsNumber | NodeBytecodeUsesAsOther);
             break;
         }
index be494c6..794566f 100644 (file)
@@ -2177,7 +2177,7 @@ bool ByteCodeParser::handleConstantInternalFunction(
         if (argumentCountIncludingThis <= 1)
             result = jsConstant(m_vm->smallStrings.emptyString());
         else
-            result = addToGraph(ToString, get(virtualRegisterForArgument(1, registerOffset)));
+            result = addToGraph(CallStringConstructor, get(virtualRegisterForArgument(1, registerOffset)));
         
         if (kind == CodeForConstruct)
             result = addToGraph(NewStringObject, OpInfo(function->globalObject()->stringObjectStructure()), result);
index 6194e98..49766c0 100644 (file)
@@ -897,6 +897,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         return;
         
     case ToString:
+    case CallStringConstructor:
         switch (node->child1().useKind()) {
         case StringObjectUse:
         case StringOrStringObjectUse:
index adee2f5..6ed0fbd 100644 (file)
@@ -143,6 +143,7 @@ bool doesGC(Graph& graph, Node* node)
     case LogicalNot:
     case ToPrimitive:
     case ToString:
+    case CallStringConstructor:
     case In:
     case Jump:
     case Branch:
index 537faac..9e53a19 100644 (file)
@@ -767,8 +767,9 @@ private:
             break;
         }
             
-        case ToString: {
-            fixupToString(node);
+        case ToString:
+        case CallStringConstructor: {
+            fixupToStringOrCallStringConstructor(node);
             break;
         }
             
@@ -1363,7 +1364,7 @@ private:
         }
     }
     
-    void fixupToString(Node* node)
+    void fixupToStringOrCallStringConstructor(Node* node)
     {
         if (node->child1()->shouldSpeculateString()) {
             fixEdge<StringUse>(node->child1());
@@ -1424,7 +1425,7 @@ private:
                 m_indexInBlock, SpecString, ToString, node->origin, Edge(toPrimitive));
             
             fixupToPrimitive(toPrimitive);
-            fixupToString(toString);
+            fixupToStringOrCallStringConstructor(toString);
             
             right.setNode(toString);
         }
index 5dc1eb6..1e79d1a 100644 (file)
@@ -261,6 +261,7 @@ namespace JSC { namespace DFG {
     macro(LogicalNot, NodeResultBoolean) \
     macro(ToPrimitive, NodeResultJS | NodeMustGenerate) \
     macro(ToString, NodeResultJS | NodeMustGenerate) \
+    macro(CallStringConstructor, NodeResultJS | NodeMustGenerate) \
     macro(NewStringObject, NodeResultJS) \
     macro(MakeRope, NodeResultJS) \
     macro(In, NodeResultBoolean | NodeMustGenerate) \
index 7614e2b..a0726c1 100644 (file)
@@ -1006,6 +1006,22 @@ JSCell* JIT_OPERATION operationToString(ExecState* exec, EncodedJSValue value)
     return JSValue::decode(value).toString(exec);
 }
 
+JSCell* JIT_OPERATION operationCallStringConstructorOnCell(ExecState* exec, JSCell* cell)
+{
+    VM& vm = exec->vm();
+    NativeCallFrameTracer tracer(&vm, exec);
+
+    return stringConstructor(exec, cell);
+}
+
+JSCell* JIT_OPERATION operationCallStringConstructor(ExecState* exec, EncodedJSValue value)
+{
+    VM& vm = exec->vm();
+    NativeCallFrameTracer tracer(&vm, exec);
+
+    return stringConstructor(exec, JSValue::decode(value));
+}
+
 JSCell* JIT_OPERATION operationMakeRope2(ExecState* exec, JSString* left, JSString* right)
 {
     VM& vm = exec->vm();
index d13eccf..1d5a792 100644 (file)
@@ -121,6 +121,8 @@ JSString* JIT_OPERATION operationSingleCharacterString(ExecState*, int32_t);
 JSCell* JIT_OPERATION operationNewStringObject(ExecState*, JSString*, Structure*);
 JSCell* JIT_OPERATION operationToStringOnCell(ExecState*, JSCell*);
 JSCell* JIT_OPERATION operationToString(ExecState*, EncodedJSValue);
+JSCell* JIT_OPERATION operationCallStringConstructorOnCell(ExecState*, JSCell*);
+JSCell* JIT_OPERATION operationCallStringConstructor(ExecState*, EncodedJSValue);
 JSCell* JIT_OPERATION operationMakeRope2(ExecState*, JSString*, JSString*);
 JSCell* JIT_OPERATION operationMakeRope3(ExecState*, JSString*, JSString*, JSString*);
 char* JIT_OPERATION operationFindSwitchImmTargetForDouble(ExecState*, EncodedJSValue, size_t tableIndex);
index 0818fa3..18e56b6 100644 (file)
@@ -480,6 +480,7 @@ private:
             break;
         }
         case StringCharAt:
+        case CallStringConstructor:
         case ToString:
         case MakeRope: {
             changed |= setPrediction(SpecString);
index f6676de..be243d7 100644 (file)
@@ -219,6 +219,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
     case LogicalNot:
     case ToPrimitive:
     case ToString:
+    case CallStringConstructor:
     case NewStringObject:
     case MakeRope:
     case In:
index 34c540a..4e5af17 100644 (file)
@@ -4848,7 +4848,7 @@ GPRReg SpeculativeJIT::temporaryRegisterForPutByVal(GPRTemporary& temporary, Arr
     return temporary.gpr();
 }
 
-void SpeculativeJIT::compileToStringOnCell(Node* node)
+void SpeculativeJIT::compileToStringOrCallStringConstructorOnCell(Node* node)
 {
     SpeculateCellOperand op1(this, node->child1());
     GPRReg op1GPR = op1.gpr();
@@ -4906,7 +4906,12 @@ void SpeculativeJIT::compileToStringOnCell(Node* node)
             done = m_jit.jump();
             needCall.link(&m_jit);
         }
-        callOperation(operationToStringOnCell, resultGPR, op1GPR);
+        if (node->op() == ToString)
+            callOperation(operationToStringOnCell, resultGPR, op1GPR);
+        else {
+            ASSERT(node->op() == CallStringConstructor);
+            callOperation(operationCallStringConstructorOnCell, resultGPR, op1GPR);
+        }
         if (done.isSet())
             done.link(&m_jit);
         cellResult(resultGPR, node);
index 9e064b2..605f2bc 100644 (file)
@@ -2138,7 +2138,7 @@ public:
     void emitSwitchString(Node*, SwitchData*);
     void emitSwitch(Node*);
     
-    void compileToStringOnCell(Node*);
+    void compileToStringOrCallStringConstructorOnCell(Node*);
     void compileNewStringObject(Node*);
     
     void compileNewTypedArray(Node*);
index 6a0b3a4..a76da85 100644 (file)
@@ -3098,7 +3098,8 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
         
-    case ToString: {
+    case ToString:
+    case CallStringConstructor: {
         if (node->child1().useKind() == UntypedUse) {
             JSValueOperand op1(this, node->child1());
             GPRReg op1PayloadGPR = op1.payloadGPR();
@@ -3118,14 +3119,19 @@ void SpeculativeJIT::compile(Node* node)
                 slowPath1.link(&m_jit);
                 slowPath2.link(&m_jit);
             }
-            callOperation(operationToString, resultGPR, op1TagGPR, op1PayloadGPR);
+            if (op == ToString)
+                callOperation(operationToString, resultGPR, op1TagGPR, op1PayloadGPR);
+            else {
+                ASSERT(op == CallStringConstructor);
+                callOperation(operationCallStringConstructor, resultGPR, op1TagGPR, op1PayloadGPR);
+            }
             if (done.isSet())
                 done.link(&m_jit);
             cellResult(resultGPR, node);
             break;
         }
         
-        compileToStringOnCell(node);
+        compileToStringOrCallStringConstructorOnCell(node);
         break;
     }
         
index 0c67e54..688b65d 100644 (file)
@@ -3185,7 +3185,8 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
         
-    case ToString: {
+    case ToString:
+    case CallStringConstructor: {
         if (node->child1().useKind() == UntypedUse) {
             JSValueOperand op1(this, node->child1());
             GPRReg op1GPR = op1.gpr();
@@ -3204,14 +3205,19 @@ void SpeculativeJIT::compile(Node* node)
                 slowPath1.link(&m_jit);
                 slowPath2.link(&m_jit);
             }
-            callOperation(operationToString, resultGPR, op1GPR);
+            if (op == ToString)
+                callOperation(operationToString, resultGPR, op1GPR);
+            else {
+                ASSERT(op == CallStringConstructor);
+                callOperation(operationCallStringConstructor, resultGPR, op1GPR);
+            }
             if (done.isSet())
                 done.link(&m_jit);
             cellResult(resultGPR, node);
             break;
         }
         
-        compileToStringOnCell(node);
+        compileToStringOrCallStringConstructorOnCell(node);
         break;
     }
         
index 4c8276b..28646d1 100644 (file)
@@ -111,6 +111,7 @@ public:
                     break;
                     
                 case ToString:
+                case CallStringConstructor:
                     registerStructure(m_graph.globalObjectFor(node->origin.semantic)->stringObjectStructure());
                     break;
                     
index 85de49b..3cd9024 100644 (file)
@@ -146,6 +146,7 @@ inline CapabilityLevel canCompile(Node* node)
     case GetCallee:
     case GetArgumentCount:
     case ToString:
+    case CallStringConstructor:
     case MakeRope:
     case NewArrayWithSize:
     case GetById:
index cea5099..38b8ab5 100644 (file)
@@ -639,7 +639,8 @@ private:
             compileReallocatePropertyStorage();
             break;
         case ToString:
-            compileToString();
+        case CallStringConstructor:
+            compileToStringOrCallStringConstructor();
             break;
         case ToPrimitive:
             compileToPrimitive();
@@ -3288,7 +3289,7 @@ private:
                 object, oldStorage, transition->previous, transition->next));
     }
     
-    void compileToString()
+    void compileToStringOrCallStringConstructor()
     {
         switch (m_node->child1().useKind()) {
         case StringObjectUse: {
@@ -3356,9 +3357,9 @@ private:
             m_out.appendTo(notString, continuation);
             LValue operation;
             if (m_node->child1().useKind() == CellUse)
-                operation = m_out.operation(operationToStringOnCell);
+                operation = m_out.operation(m_node->op() == ToString ? operationToStringOnCell : operationCallStringConstructorOnCell);
             else
-                operation = m_out.operation(operationToString);
+                operation = m_out.operation(m_node->op() == ToString ? operationToString : operationCallStringConstructor);
             ValueFromBlock convertedResult = m_out.anchor(vmCall(operation, m_callFrame, value));
             m_out.jump(continuation);
             
index becacbe..a39973e 100644 (file)
@@ -106,14 +106,18 @@ ConstructType StringConstructor::getConstructData(JSCell*, ConstructData& constr
     return ConstructTypeHost;
 }
 
+JSCell* stringConstructor(ExecState* exec, JSValue argument)
+{
+    if (argument.isSymbol())
+        return jsNontrivialString(exec, asSymbol(argument)->descriptiveString());
+    return argument.toString(exec);
+}
+
 static EncodedJSValue JSC_HOST_CALL callStringConstructor(ExecState* exec)
 {
     if (!exec->argumentCount())
         return JSValue::encode(jsEmptyString(exec));
-    JSValue argument = exec->uncheckedArgument(0);
-    if (argument.isSymbol())
-        return JSValue::encode(jsString(exec, asSymbol(argument)->descriptiveString()));
-    return JSValue::encode(argument.toString(exec));
+    return JSValue::encode(stringConstructor(exec, exec->uncheckedArgument(0)));
 }
 
 CallType StringConstructor::getCallData(JSCell*, CallData& callData)
index e8bcefd..0086a6b 100644 (file)
@@ -56,8 +56,9 @@ private:
 
     static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
 };
-    
+
 JSCell* JSC_HOST_CALL stringFromCharCode(ExecState*, int32_t);
+JSCell* stringConstructor(ExecState*, JSValue);
 
 } // namespace JSC
 
diff --git a/Source/JavaScriptCore/tests/stress/symbol-and-string-constructor.js b/Source/JavaScriptCore/tests/stress/symbol-and-string-constructor.js
new file mode 100644 (file)
index 0000000..4a88e86
--- /dev/null
@@ -0,0 +1,10 @@
+function performString(value) {
+    return String(value);
+}
+noInline(performString);
+
+for (var i = 0; i < 10000; ++i) {
+    var result = performString(Symbol.iterator);
+    if (result !== 'Symbol(Symbol.iterator)')
+        throw new Error('bad value: ' + result);
+}