Title: [214752] releases/WebKitGTK/webkit-2.16
Revision
214752
Author
carlo...@webkit.org
Date
2017-04-03 01:15:27 -0700 (Mon, 03 Apr 2017)

Log Message

Merge r214028 - [DFG] ToString operation should have fixup for primitives to say this node does not have side effects
https://bugs.webkit.org/show_bug.cgi?id=169544

Reviewed by Saam Barati.

JSTests:

* microbenchmarks/template-string-array.js: Added.
(test):
* stress/to-string-non-cell-use.js: Added.
(shouldBe):
(shouldThrow):

Source/_javascript_Core:

Our DFG ToString only considers well about String operands. While ToString(non cell operand) does not have
any side effect, it is not modeled well in DFG.

This patch introduces a fixup for ToString with NonCellUse edge. If this edge is set, ToString does not
clobber things (like ToLowerCase, producing String). And ToString(NonCellUse) allows us to perform CSE!

Our microbenchmark shows 32.9% improvement due to dropped GetButterfly and CSE for ToString().

                                    baseline                  patched

    template-string-array       12.6284+-0.2766     ^      9.4998+-0.2295        ^ definitely 1.3293x faster

And SixSpeed template_string.es6 shows 16.68x performance improvement due to LICM onto this non-side-effectful ToString().

                                  baseline                  patched

    template_string.es6     3229.7343+-40.5705    ^    193.6077+-36.3349       ^ definitely 16.6818x faster

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOnCell):
(JSC::DFG::SpeculativeJIT::speculateNotCell):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructor):
(JSC::FTL::DFG::LowerDFGToB3::lowNotCell):
(JSC::FTL::DFG::LowerDFGToB3::speculateNotCell):

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.16/JSTests/ChangeLog (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/JSTests/ChangeLog	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/JSTests/ChangeLog	2017-04-03 08:15:27 UTC (rev 214752)
@@ -1,3 +1,16 @@
+2017-03-15  Yusuke Suzuki  <utatane....@gmail.com>
+
+        [DFG] ToString operation should have fixup for primitives to say this node does not have side effects
+        https://bugs.webkit.org/show_bug.cgi?id=169544
+
+        Reviewed by Saam Barati.
+
+        * microbenchmarks/template-string-array.js: Added.
+        (test):
+        * stress/to-string-non-cell-use.js: Added.
+        (shouldBe):
+        (shouldThrow):
+
 2017-03-13  Caio Lima  <ticaiol...@gmail.com>
 
         [JSC] It should be possible create a label named let when parsing Statement in non strict mode

Added: releases/WebKitGTK/webkit-2.16/JSTests/microbenchmarks/template-string-array.js (0 => 214752)


--- releases/WebKitGTK/webkit-2.16/JSTests/microbenchmarks/template-string-array.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.16/JSTests/microbenchmarks/template-string-array.js	2017-04-03 08:15:27 UTC (rev 214752)
@@ -0,0 +1,9 @@
+var array = [1, 2, 3];
+function test()
+{
+    return `${array[0]}, ${array[1]}, ${array[2]}, ${array[0]}, ${array[1]}, ${array[2]}`;
+}
+noInline(test);
+
+for (var i = 0; i < 1e5; ++i)
+    test();

Added: releases/WebKitGTK/webkit-2.16/JSTests/stress/to-string-non-cell-use.js (0 => 214752)


--- releases/WebKitGTK/webkit-2.16/JSTests/stress/to-string-non-cell-use.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.16/JSTests/stress/to-string-non-cell-use.js	2017-04-03 08:15:27 UTC (rev 214752)
@@ -0,0 +1,43 @@
+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)}`);
+}
+
+function toString(value)
+{
+    return `${value}`;
+}
+noInline(toString);
+
+for (var i = 0; i < 1e4; ++i) {
+    shouldBe(toString(i), i + "");
+    shouldBe(toString(null), "null");
+    shouldBe(toString(undefined), "undefined");
+    shouldBe(toString(10.5), "10.5");
+    shouldBe(toString(-10.5), "-10.5");
+    shouldBe(toString(true), "true");
+    shouldBe(toString(false), "false");
+    shouldBe(toString(0 / 0), "NaN");
+}
+
+shouldBe(toString("HELLO"), "HELLO");
+shouldThrow(() => {
+    toString(Symbol("Cocoa"));
+}, `TypeError: Cannot convert a symbol to a string`);

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog	2017-04-03 08:15:27 UTC (rev 214752)
@@ -1,3 +1,47 @@
+2017-03-15  Yusuke Suzuki  <utatane....@gmail.com>
+
+        [DFG] ToString operation should have fixup for primitives to say this node does not have side effects
+        https://bugs.webkit.org/show_bug.cgi?id=169544
+
+        Reviewed by Saam Barati.
+
+        Our DFG ToString only considers well about String operands. While ToString(non cell operand) does not have
+        any side effect, it is not modeled well in DFG.
+
+        This patch introduces a fixup for ToString with NonCellUse edge. If this edge is set, ToString does not
+        clobber things (like ToLowerCase, producing String). And ToString(NonCellUse) allows us to perform CSE!
+
+        Our microbenchmark shows 32.9% improvement due to dropped GetButterfly and CSE for ToString().
+
+                                            baseline                  patched
+
+            template-string-array       12.6284+-0.2766     ^      9.4998+-0.2295        ^ definitely 1.3293x faster
+
+        And SixSpeed template_string.es6 shows 16.68x performance improvement due to LICM onto this non-side-effectful ToString().
+
+                                          baseline                  patched
+
+            template_string.es6     3229.7343+-40.5705    ^    193.6077+-36.3349       ^ definitely 16.6818x faster
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOnCell):
+        (JSC::DFG::SpeculativeJIT::speculateNotCell):
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructor):
+        (JSC::FTL::DFG::LowerDFGToB3::lowNotCell):
+        (JSC::FTL::DFG::LowerDFGToB3::speculateNotCell):
+
 2017-03-15  Daniel Ehrenberg  <little...@chromium.org>
 
         Switch back to ISO 4217 for Intl CurrencyDigits data

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2017-04-03 08:15:27 UTC (rev 214752)
@@ -1858,6 +1858,7 @@
                 m_graph.registerStructure(m_graph.globalObjectFor(node->origin.semantic)->stringObjectStructure()));
             break;
         case StringOrStringObjectUse:
+        case NotCellUse:
             break;
         case CellUse:
         case UntypedUse:

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGClobberize.h (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGClobberize.h	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGClobberize.h	2017-04-03 08:15:27 UTC (rev 214752)
@@ -1389,6 +1389,10 @@
             read(World);
             write(Heap);
             return;
+
+        case NotCellUse:
+            def(PureValue(node));
+            return;
             
         default:
             RELEASE_ASSERT_NOT_REACHED();

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2017-04-03 08:15:27 UTC (rev 214752)
@@ -2210,6 +2210,16 @@
             fixEdge<CellUse>(node->child1());
             return;
         }
+
+        // ToString(Symbol) throws an error. So if the child1 can include Symbols,
+        // we need to care about it in the clobberize. In the following case,
+        // since NotCellUse edge filter is used and this edge filters Symbols,
+        // we can say that ToString never throws an error!
+        if (node->child1()->shouldSpeculateNotCell()) {
+            fixEdge<NotCellUse>(node->child1());
+            node->clearFlags(NodeMustGenerate);
+            return;
+        }
     }
 
     bool attemptToMakeFastStringAdd(Node* node)

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2017-04-03 08:15:27 UTC (rev 214752)
@@ -7846,6 +7846,61 @@
 
 void SpeculativeJIT::compileToStringOrCallStringConstructorOnCell(Node* node)
 {
+    if (node->child1().useKind() == NotCellUse) {
+        JSValueOperand op1(this, node->child1(), ManualOperandSpeculation);
+        JSValueRegs op1Regs = op1.jsValueRegs();
+
+        GPRFlushedCallResult result(this);
+        GPRReg resultGPR = result.gpr();
+
+        speculateNotCell(node->child1(), op1Regs);
+
+        flushRegisters();
+
+        if (node->op() == ToString)
+            callOperation(operationToString, resultGPR, op1Regs);
+        else {
+            ASSERT(node->op() == CallStringConstructor);
+            callOperation(operationCallStringConstructor, resultGPR, op1Regs);
+        }
+        m_jit.exceptionCheck();
+        cellResult(resultGPR, node);
+        return;
+    }
+
+    if (node->child1().useKind() == UntypedUse) {
+        JSValueOperand op1(this, node->child1());
+        JSValueRegs op1Regs = op1.jsValueRegs();
+        GPRReg op1PayloadGPR = op1Regs.payloadGPR();
+
+        GPRFlushedCallResult result(this);
+        GPRReg resultGPR = result.gpr();
+
+        flushRegisters();
+
+        JITCompiler::Jump done;
+        if (node->child1()->prediction() & SpecString) {
+            JITCompiler::Jump slowPath1 = m_jit.branchIfNotCell(op1.jsValueRegs());
+            JITCompiler::Jump slowPath2 = m_jit.branchIfNotString(op1PayloadGPR);
+            m_jit.move(op1PayloadGPR, resultGPR);
+            done = m_jit.jump();
+            slowPath1.link(&m_jit);
+            slowPath2.link(&m_jit);
+        }
+        if (node->op() == ToString)
+            callOperation(operationToString, resultGPR, op1Regs);
+        else {
+            ASSERT(node->op() == CallStringConstructor);
+            callOperation(operationCallStringConstructor, resultGPR, op1Regs);
+        }
+        m_jit.exceptionCheck();
+        if (done.isSet())
+            done.link(&m_jit);
+        cellResult(resultGPR, node);
+        return;
+    }
+
+
     SpeculateCellOperand op1(this, node->child1());
     GPRReg op1GPR = op1.gpr();
     
@@ -8454,6 +8509,11 @@
     speculateSymbol(edge, operand.gpr());
 }
 
+void SpeculativeJIT::speculateNotCell(Edge edge, JSValueRegs regs)
+{
+    DFG_TYPE_CHECK(regs, edge, ~SpecCell, m_jit.branchIfCell(regs));
+}
+
 void SpeculativeJIT::speculateNotCell(Edge edge)
 {
     if (!needsTypeCheck(edge, ~SpecCell))
@@ -8460,7 +8520,7 @@
         return;
     
     JSValueOperand operand(this, edge, ManualOperandSpeculation); 
-    typeCheck(operand.jsValueRegs(), edge, ~SpecCell, m_jit.branchIfCell(operand.jsValueRegs()));
+    speculateNotCell(edge, operand.jsValueRegs());
 }
 
 void SpeculativeJIT::speculateOther(Edge edge)

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2017-04-03 08:15:27 UTC (rev 214752)
@@ -1628,6 +1628,11 @@
         m_jit.setupArgumentsWithExecState(arg1);
         return appendCallSetResult(operation, result);
     }
+    JITCompiler::Call callOperation(C_JITOperation_EJ operation, GPRReg result, JSValueRegs arg1)
+    {
+        m_jit.setupArgumentsWithExecState(arg1.gpr());
+        return appendCallSetResult(operation, result);
+    }
     JITCompiler::Call callOperation(C_JITOperation_EJJ operation, GPRReg result, GPRReg arg1, GPRReg arg2)
     {
         m_jit.setupArgumentsWithExecState(arg1, arg2);
@@ -2908,6 +2913,7 @@
     void speculateStringOrStringObject(Edge);
     void speculateSymbol(Edge, GPRReg cell);
     void speculateSymbol(Edge);
+    void speculateNotCell(Edge, JSValueRegs);
     void speculateNotCell(Edge);
     void speculateOther(Edge);
     void speculateMisc(Edge, JSValueRegs);

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2017-04-03 08:15:27 UTC (rev 214752)
@@ -3730,38 +3730,6 @@
         
     case ToString:
     case CallStringConstructor: {
-        if (node->child1().useKind() == UntypedUse) {
-            JSValueOperand op1(this, node->child1());
-            GPRReg op1PayloadGPR = op1.payloadGPR();
-            JSValueRegs op1Regs = op1.jsValueRegs();
-            
-            GPRFlushedCallResult result(this);
-            GPRReg resultGPR = result.gpr();
-            
-            flushRegisters();
-            
-            JITCompiler::Jump done;
-            if (node->child1()->prediction() & SpecString) {
-                JITCompiler::Jump slowPath1 = m_jit.branchIfNotCell(op1.jsValueRegs());
-                JITCompiler::Jump slowPath2 = m_jit.branchIfNotString(op1PayloadGPR);
-                m_jit.move(op1PayloadGPR, resultGPR);
-                done = m_jit.jump();
-                slowPath1.link(&m_jit);
-                slowPath2.link(&m_jit);
-            }
-            if (op == ToString)
-                callOperation(operationToString, resultGPR, op1Regs);
-            else {
-                ASSERT(op == CallStringConstructor);
-                callOperation(operationCallStringConstructor, resultGPR, op1Regs);
-            }
-            m_jit.exceptionCheck();
-            if (done.isSet())
-                done.link(&m_jit);
-            cellResult(resultGPR, node);
-            break;
-        }
-        
         compileToStringOrCallStringConstructorOnCell(node);
         break;
     }

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2017-04-03 08:15:27 UTC (rev 214752)
@@ -3711,37 +3711,6 @@
         
     case ToString:
     case CallStringConstructor: {
-        if (node->child1().useKind() == UntypedUse) {
-            JSValueOperand op1(this, node->child1());
-            GPRReg op1GPR = op1.gpr();
-            
-            GPRFlushedCallResult result(this);
-            GPRReg resultGPR = result.gpr();
-            
-            flushRegisters();
-            
-            JITCompiler::Jump done;
-            if (node->child1()->prediction() & SpecString) {
-                JITCompiler::Jump slowPath1 = m_jit.branchIfNotCell(JSValueRegs(op1GPR));
-                JITCompiler::Jump slowPath2 = m_jit.branchIfNotString(op1GPR);
-                m_jit.move(op1GPR, resultGPR);
-                done = m_jit.jump();
-                slowPath1.link(&m_jit);
-                slowPath2.link(&m_jit);
-            }
-            if (op == ToString)
-                callOperation(operationToString, resultGPR, op1GPR);
-            else {
-                ASSERT(op == CallStringConstructor);
-                callOperation(operationCallStringConstructor, resultGPR, op1GPR);
-            }
-            m_jit.exceptionCheck();
-            if (done.isSet())
-                done.link(&m_jit);
-            cellResult(resultGPR, node);
-            break;
-        }
-        
         compileToStringOrCallStringConstructorOnCell(node);
         break;
     }

Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (214751 => 214752)


--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2017-04-03 08:03:33 UTC (rev 214751)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2017-04-03 08:15:27 UTC (rev 214752)
@@ -4898,10 +4898,13 @@
         }
             
         case CellUse:
+        case NotCellUse:
         case UntypedUse: {
             LValue value;
             if (m_node->child1().useKind() == CellUse)
                 value = lowCell(m_node->child1());
+            else if (m_node->child1().useKind() == NotCellUse)
+                value = lowNotCell(m_node->child1());
             else
                 value = lowJSValue(m_node->child1());
             
@@ -4912,6 +4915,8 @@
             LValue isCellPredicate;
             if (m_node->child1().useKind() == CellUse)
                 isCellPredicate = m_out.booleanTrue;
+            else if (m_node->child1().useKind() == NotCellUse)
+                isCellPredicate = m_out.booleanFalse;
             else
                 isCellPredicate = this->isCell(value, provenType(m_node->child1()));
             m_out.branch(isCellPredicate, unsure(isCell), unsure(notString));
@@ -12095,6 +12100,13 @@
         DFG_CRASH(m_graph, m_node, "Value not defined");
         return 0;
     }
+
+    LValue lowNotCell(Edge edge)
+    {
+        LValue result = lowJSValue(edge, ManualOperandSpeculation);
+        FTL_TYPE_CHECK(jsValueValue(result), edge, ~SpecCell, isCell(result));
+        return result;
+    }
     
     LValue lowStorage(Edge edge)
     {
@@ -12499,6 +12511,13 @@
     {
         lowCell(edge);
     }
+
+    void speculateNotCell(Edge edge)
+    {
+        if (!m_interpreter.needsTypeCheck(edge))
+            return;
+        lowNotCell(edge);
+    }
     
     void speculateCellOrOther(Edge edge)
     {
@@ -13017,15 +13036,6 @@
         m_out.appendTo(continuation, lastNext);
     }
     
-    void speculateNotCell(Edge edge)
-    {
-        if (!m_interpreter.needsTypeCheck(edge))
-            return;
-        
-        LValue value = lowJSValue(edge, ManualOperandSpeculation);
-        typeCheck(jsValueValue(value), edge, ~SpecCell, isCell(value));
-    }
-    
     void speculateOther(Edge edge)
     {
         if (!m_interpreter.needsTypeCheck(edge))
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to