[DFG][FTL] Make ArraySlice(0) code tight
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Mar 2018 07:12:56 +0000 (07:12 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Mar 2018 07:12:56 +0000 (07:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183590

Reviewed by Saam Barati.

JSTests:

* stress/array-slice-with-zero.js: Added.
(shouldBe):
(test):
(test2):
* stress/array-slice-zero-args.js: Added.
(shouldBe):
(test):

Source/JavaScriptCore:

This patch tightens ArraySlice code, in particular, startIndex = 0 case.

1. We support array.slice() call. This is a well-used way to clone array.
For example, underscore.js uses this technique.

2. We remove several checks if the given index value is a proven constant.

* dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::propagate):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleIntrinsicCall):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::emitPopulateSliceIndex):
(JSC::DFG::SpeculativeJIT::compileArraySlice):
We can skip some of checks if the given value is a proven constant.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileArraySlice):
Change below to belowOrEqual. It does not change meaning in the code. But it allows us
to fold BelowEqual(0, x) to true.

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

JSTests/ChangeLog
JSTests/stress/array-slice-with-zero.js [new file with mode: 0644]
JSTests/stress/array-slice-zero-args.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index 05f5d981f9595d4a121dd372f7ee7ea47fafed17..4aa1bf0567938a4de6820e5f0c424a17deb23197 100644 (file)
@@ -1,3 +1,18 @@
+2018-03-13  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Make ArraySlice(0) code tight
+        https://bugs.webkit.org/show_bug.cgi?id=183590
+
+        Reviewed by Saam Barati.
+
+        * stress/array-slice-with-zero.js: Added.
+        (shouldBe):
+        (test):
+        (test2):
+        * stress/array-slice-zero-args.js: Added.
+        (shouldBe):
+        (test):
+
 2018-03-14  Caitlin Potter  <caitp@igalia.com>
 
         [JSC] fix order of evaluation for ClassDefinitionEvaluation
diff --git a/JSTests/stress/array-slice-with-zero.js b/JSTests/stress/array-slice-with-zero.js
new file mode 100644 (file)
index 0000000..3d82f81
--- /dev/null
@@ -0,0 +1,34 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test(array)
+{
+    return array.slice(0);
+}
+noInline(test);
+
+for (var i = 0; i < 1e5; ++i) {
+    var array = [i, i, i];
+    var result = test(array);
+    shouldBe(array !== result, true);
+    shouldBe(result.length, 3);
+    for (var j = 0; j < 3; ++j)
+        shouldBe(result[j], i);
+}
+
+function test2(array, i)
+{
+    return array.slice(0, i);
+}
+noInline(test2);
+
+for (var i = 0; i < 1e5; ++i) {
+    var array = [i, i, i];
+    var result = test2(array, 2);
+    shouldBe(array !== result, true);
+    shouldBe(result.length, 2);
+    for (var j = 0; j < 2; ++j)
+        shouldBe(result[j], i);
+}
diff --git a/JSTests/stress/array-slice-zero-args.js b/JSTests/stress/array-slice-zero-args.js
new file mode 100644 (file)
index 0000000..48fe9ca
--- /dev/null
@@ -0,0 +1,19 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test(array)
+{
+    return array.slice();
+}
+noInline(test);
+
+for (var i = 0; i < 1e6; ++i) {
+    var array = [i, i, i];
+    var result = test(array);
+    shouldBe(array !== result, true);
+    shouldBe(result.length, 3);
+    for (var j = 0; j < 3; ++j)
+        shouldBe(result[j], i);
+}
index 0c322a62e844a2bb2d3a08cc692faf001b4e9ce1..1b3c8b7686c269275d03a31798f992896ae93009 100644 (file)
@@ -1,3 +1,33 @@
+2018-03-13  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Make ArraySlice(0) code tight
+        https://bugs.webkit.org/show_bug.cgi?id=183590
+
+        Reviewed by Saam Barati.
+
+        This patch tightens ArraySlice code, in particular, startIndex = 0 case.
+
+        1. We support array.slice() call. This is a well-used way to clone array.
+        For example, underscore.js uses this technique.
+
+        2. We remove several checks if the given index value is a proven constant.
+
+        * dfg/DFGBackwardsPropagationPhase.cpp:
+        (JSC::DFG::BackwardsPropagationPhase::propagate):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleIntrinsicCall):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::emitPopulateSliceIndex):
+        (JSC::DFG::SpeculativeJIT::compileArraySlice):
+        We can skip some of checks if the given value is a proven constant.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileArraySlice):
+        Change below to belowOrEqual. It does not change meaning in the code. But it allows us
+        to fold BelowEqual(0, x) to true.
+
 2018-03-19  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Drop s_exceptionInstructions static initializer
index 1d1fe03e20730797f0cd0bed39e992a9877b3616..9547f89661a25a1fa762d69c8c76769d3861b879 100644 (file)
@@ -238,10 +238,14 @@ private:
 
         case ArraySlice: {
             m_graph.varArgChild(node, 0)->mergeFlags(NodeBytecodeUsesAsValue);
-            m_graph.varArgChild(node, 1)->mergeFlags(NodeBytecodeUsesAsNumber | NodeBytecodeUsesAsOther | NodeBytecodeUsesAsInt | NodeBytecodeUsesAsArrayIndex);
-            if (node->numChildren() == 3)
+
+            if (node->numChildren() == 2)
+                m_graph.varArgChild(node, 1)->mergeFlags(NodeBytecodeUsesAsValue);
+            else if (node->numChildren() == 3) {
+                m_graph.varArgChild(node, 1)->mergeFlags(NodeBytecodeUsesAsNumber | NodeBytecodeUsesAsOther | NodeBytecodeUsesAsInt | NodeBytecodeUsesAsArrayIndex);
                 m_graph.varArgChild(node, 2)->mergeFlags(NodeBytecodeUsesAsValue);
-            else {
+            } else if (node->numChildren() == 4) {
+                m_graph.varArgChild(node, 1)->mergeFlags(NodeBytecodeUsesAsNumber | NodeBytecodeUsesAsOther | NodeBytecodeUsesAsInt | NodeBytecodeUsesAsArrayIndex);
                 m_graph.varArgChild(node, 2)->mergeFlags(NodeBytecodeUsesAsNumber | NodeBytecodeUsesAsOther | NodeBytecodeUsesAsInt | NodeBytecodeUsesAsArrayIndex);
                 m_graph.varArgChild(node, 3)->mergeFlags(NodeBytecodeUsesAsValue);
             }
index a563d78043cdf8d12255a4d0b2f20b4ced931878..0fefa547e33eee22063537d0e461a994fbec49af 100644 (file)
@@ -2241,7 +2241,7 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
             return false;
         }
 #endif
-        if (argumentCountIncludingThis < 2)
+        if (argumentCountIncludingThis < 1)
             return false;
 
         if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadConstantCache)
@@ -2303,7 +2303,8 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
                 addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(structureSet)), array);
 
                 addVarArgChild(array);
-                addVarArgChild(get(virtualRegisterForArgument(1, registerOffset))); // Start index.
+                if (argumentCountIncludingThis >= 2)
+                    addVarArgChild(get(virtualRegisterForArgument(1, registerOffset))); // Start index.
                 if (argumentCountIncludingThis >= 3)
                     addVarArgChild(get(virtualRegisterForArgument(2, registerOffset))); // End index.
                 addVarArgChild(addToGraph(GetButterfly, array));
index 142943deec9db4b860b5cee946a8d65f69c8e1dd..0485ca85636c24b4739ab4a2f765b44c0ad2f719 100644 (file)
@@ -1075,9 +1075,11 @@ private:
 
         case ArraySlice: {
             fixEdge<KnownCellUse>(m_graph.varArgChild(node, 0));
-            fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
-            if (node->numChildren() == 4)
-                fixEdge<Int32Use>(m_graph.varArgChild(node, 2));
+            if (node->numChildren() >= 3) {
+                fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
+                if (node->numChildren() == 4)
+                    fixEdge<Int32Use>(m_graph.varArgChild(node, 2));
+            }
             break;
         }
 
index 8544ceb9fd305c7c60ea067f976c17d48eee1efc..c0afd5399e9b1b871b75e3190a5c6c9a22432152 100644 (file)
@@ -7619,9 +7619,32 @@ void SpeculativeJIT::compileGetRestLength(Node* node)
 
 void SpeculativeJIT::emitPopulateSliceIndex(Edge& target, GPRReg length, GPRReg result)
 {
+    if (target->isInt32Constant()) {
+        int32_t value = target->asInt32();
+        if (value == 0) {
+            m_jit.move(TrustedImm32(0), result);
+            return;
+        }
+
+        MacroAssembler::JumpList done;
+        if (value > 0) {
+            m_jit.move(TrustedImm32(value), result);
+            done.append(m_jit.branch32(MacroAssembler::BelowOrEqual, result, length));
+            m_jit.move(length, result);
+        } else {
+            ASSERT(value != 0);
+            m_jit.move(length, result);
+            done.append(m_jit.branchAdd32(MacroAssembler::PositiveOrZero, TrustedImm32(value), result));
+            m_jit.move(TrustedImm32(0), result);
+        }
+        done.link(&m_jit);
+        return;
+    }
+
     SpeculateInt32Operand index(this, target);
     GPRReg indexGPR = index.gpr();
     MacroAssembler::JumpList done;
+
     auto isPositive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, indexGPR, TrustedImm32(0));
     m_jit.move(length, result);
     done.append(m_jit.branchAdd32(MacroAssembler::PositiveOrZero, indexGPR, result));
@@ -7643,14 +7666,17 @@ void SpeculativeJIT::compileArraySlice(Node* node)
     JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node->origin.semantic);
 
     GPRTemporary temp(this);
-    StorageOperand storage(this, node->numChildren() == 3 ? m_jit.graph().varArgChild(node, 2) : m_jit.graph().varArgChild(node, 3));
+    StorageOperand storage(this, m_jit.graph().varArgChild(node, node->numChildren() - 1));
     GPRTemporary result(this);
     
     GPRReg storageGPR = storage.gpr();
     GPRReg resultGPR = result.gpr();
     GPRReg tempGPR = temp.gpr();
 
-    {
+    if (node->numChildren() == 2)
+        m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), tempGPR);
+    else {
+        ASSERT(node->numChildren() == 3 || node->numChildren() == 4);
         GPRTemporary tempLength(this);
         GPRReg lengthGPR = tempLength.gpr();
         m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), lengthGPR);
@@ -7660,17 +7686,22 @@ void SpeculativeJIT::compileArraySlice(Node* node)
         else
             m_jit.move(lengthGPR, tempGPR);
 
-        GPRTemporary tempStartIndex(this);
-        GPRReg startGPR = tempStartIndex.gpr();
-        emitPopulateSliceIndex(m_jit.graph().varArgChild(node, 1), lengthGPR, startGPR);
+        if (m_jit.graph().varArgChild(node, 1)->isInt32Constant() && m_jit.graph().varArgChild(node, 1)->asInt32() == 0) {
+            // Do nothing for array.slice(0, end) or array.slice(0) cases.
+            // `tempGPR` already points to the size of a newly created array.
+        } else {
+            GPRTemporary tempStartIndex(this);
+            GPRReg startGPR = tempStartIndex.gpr();
+            emitPopulateSliceIndex(m_jit.graph().varArgChild(node, 1), lengthGPR, startGPR);
 
-        auto tooBig = m_jit.branch32(MacroAssembler::Above, startGPR, tempGPR);
-        m_jit.sub32(startGPR, tempGPR); // the size of the array we'll make.
-        auto done = m_jit.jump();
+            auto tooBig = m_jit.branch32(MacroAssembler::Above, startGPR, tempGPR);
+            m_jit.sub32(startGPR, tempGPR); // the size of the array we'll make.
+            auto done = m_jit.jump();
 
-        tooBig.link(&m_jit);
-        m_jit.move(TrustedImm32(0), tempGPR);
-        done.link(&m_jit);
+            tooBig.link(&m_jit);
+            m_jit.move(TrustedImm32(0), tempGPR);
+            done.link(&m_jit);
+        }
     }
 
 
@@ -7757,12 +7788,17 @@ void SpeculativeJIT::compileArraySlice(Node* node)
     GPRTemporary temp4(this);
     GPRReg loadIndex = temp4.gpr();
 
-    m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), tempValue);
-    if (node->numChildren() == 4)
-        emitPopulateSliceIndex(m_jit.graph().varArgChild(node, 2), tempValue, tempGPR);
-    else
-        m_jit.move(tempValue, tempGPR);
-    emitPopulateSliceIndex(m_jit.graph().varArgChild(node, 1), tempValue, loadIndex);
+    if (node->numChildren() == 2) {
+        m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), tempGPR);
+        m_jit.move(TrustedImm32(0), loadIndex);
+    } else {
+        m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), tempValue);
+        if (node->numChildren() == 4)
+            emitPopulateSliceIndex(m_jit.graph().varArgChild(node, 2), tempValue, tempGPR);
+        else
+            m_jit.move(tempValue, tempGPR);
+        emitPopulateSliceIndex(m_jit.graph().varArgChild(node, 1), tempValue, loadIndex);
+    }
 
     GPRTemporary temp5(this);
     GPRReg storeIndex = temp5.gpr();
index 4a94e67cbf51a144a65536519100c82c4b9e328e..de4fe13adbde507bc5498e56fe28d6d01587f6ee 100644 (file)
@@ -4690,20 +4690,28 @@ private:
     {
         JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
 
-        LValue sourceStorage = lowStorage(m_node->numChildren() == 3 ? m_graph.varArgChild(m_node, 2) : m_graph.varArgChild(m_node, 3));
+        LValue sourceStorage = lowStorage(m_graph.varArgChild(m_node, m_node->numChildren() - 1));
         LValue inputLength = m_out.load32(sourceStorage, m_heaps.Butterfly_publicLength);
-        LValue start = lowInt32(m_graph.varArgChild(m_node, 1));
-        LValue end = nullptr;
-        if (m_node->numChildren() != 3)
-            end = lowInt32(m_graph.varArgChild(m_node, 2));
 
-        auto range = populateSliceRange(start, end, inputLength);
-        LValue startIndex = range.first;
-        LValue endBoundary = range.second;
+        LValue startIndex = nullptr;
+        LValue resultLength = nullptr;
+        if (m_node->numChildren() == 2) {
+            startIndex = m_out.constInt32(0);
+            resultLength = inputLength;
+        } else {
+            LValue start = lowInt32(m_graph.varArgChild(m_node, 1));
+            LValue end = nullptr;
+            if (m_node->numChildren() != 3)
+                end = lowInt32(m_graph.varArgChild(m_node, 2));
+
+            auto range = populateSliceRange(start, end, inputLength);
+            startIndex = range.first;
+            LValue endBoundary = range.second;
 
-        LValue resultLength = m_out.select(m_out.below(startIndex, endBoundary),
-            m_out.sub(endBoundary, startIndex),
-            m_out.constInt32(0));
+            resultLength = m_out.select(m_out.belowOrEqual(startIndex, endBoundary),
+                m_out.sub(endBoundary, startIndex),
+                m_out.constInt32(0));
+        }
 
         ArrayValues arrayResult;
         {