Changeset 222675 in webkit for trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

Timestamp:

Sep 29, 2017, 6:16:52 PM (8 years ago)

Author:

Yusuke Suzuki

Message:

[DFG] Support ArrayPush with multiple args
​https://bugs.webkit.org/show_bug.cgi?id=175823

Reviewed by Saam Barati.

JSTests:

• microbenchmarks/array-push-0.js: Added.

(arrayPush0):

• microbenchmarks/array-push-1.js: Added.

(arrayPush1):

• microbenchmarks/array-push-2.js: Added.

(arrayPush2):

• microbenchmarks/array-push-3.js: Added.

(arrayPush3):

• stress/array-push-multiple-contiguous.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-double-nan.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-double.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-int32.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-contiguous.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-double.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-int32.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-many-storage.js: Added.

(shouldBe):
(test):

• stress/array-push-multiple-storage.js: Added.

(shouldBe):
(test):

• stress/array-push-with-force-exit.js: Added.

(target.createBuiltin):

Source/JavaScriptCore:

Reviewed by Saam Barati.

This patch implements ArrayPush(with multiple arguments) in DFG and FTL. Previously, they are not handled
by ArrayPush. Then they go to generic direct call to Array#push and it does in slow path. This patch
extends ArrayPush to push multiple arguments in a bulk push manner.

The problem of ArrayPush is that we need to perform ArrayPush atomically: If OSR exit occurs in the middle
of ArrayPush, we incorrectly push pushed elements twice. Once we start pushing values, we should not exit.
But we do not want to iterate elements twice, once for type checks and once for actually pushing it. It
could move elements between registers and memory back and forth.

This patch achieves the above goal by separating type checks from ArrayPush. When starting ArrayPush, type
checks for elements are already done by separately emitted Check nodes.

We also add JSArray::pushInline for DFG operations just calling JSArray::push. And we also use it in
arrayProtoFuncPush's fast path.

This patch significantly improves performance of push(multiple args).

baseline patched

Microbenchmarks:

array-push-0 461.8455+-28.9995 ^{151.3438+-6.5653} definitely 3.0516x faster
array-push-1 133.8845+-7.0349 ? 136.1775+-5.8327 ? might be 1.0171x slower
array-push-2 675.6555+-13.4645 ^{145.8747+-6.4621} definitely 4.6318x faster
array-push-3 849.5284+-15.2540 ^{253.4421+-9.1249} definitely 3.3520x faster

baseline patched

SixSpeed:

spread-literal.es5 90.3482+-6.6514 ^{24.8123+-2.3304} definitely 3.6413x faster

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::handleIntrinsicCall):

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

• dfg/DFGNodeType.h:
• dfg/DFGOperations.cpp:
• dfg/DFGOperations.h:
• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileArrayPush):

• dfg/DFGSpeculativeJIT.h:

(JSC::DFG::SpeculativeJIT::callOperation):

• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGStoreBarrierInsertionPhase.cpp:
• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileArrayPush):

• jit/JITOperations.h:
• runtime/ArrayPrototype.cpp:

(JSC::arrayProtoFuncPush):

• runtime/JSArray.cpp:

(JSC::JSArray::push):

• runtime/JSArray.h:
• runtime/JSArrayInlines.h:

(JSC::JSArray::pushInline):

File:

• trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -1009 +1009 @@
             // array-related - so if refine() turned this into a "Generic" ArrayPush then
             // that would break things.
-            node->setArrayMode(
-                node->arrayMode().refine(
-                    m_graph, node,
-                    node->child1()->prediction() & SpecCell,
-                    SpecInt32Only,
-                    node->child2()->prediction()));
-            blessArrayOperation(node->child1(), Edge(), node->child3());
-            fixEdge<KnownCellUse>(node->child1());
-            
-            switch (node->arrayMode().type()) {
-            case Array::Int32:
-                fixEdge<Int32Use>(node->child2());
-                break;
-            case Array::Double:
-                fixEdge<DoubleRepRealUse>(node->child2());
-                break;
-            case Array::Contiguous:
-            case Array::ArrayStorage:
-                speculateForBarrier(node->child2());
-                break;
-            default:
-                break;
+            Edge& storageEdge = m_graph.varArgChild(node, 0);
+            Edge& arrayEdge = m_graph.varArgChild(node, 1);
+            unsigned elementOffset = 2;
+            unsigned elementCount = node->numChildren() - elementOffset;
+            for (unsigned i = 0; i < elementCount; ++i) {
+                Edge& element = m_graph.varArgChild(node, i + elementOffset);
+                node->setArrayMode(
+                    node->arrayMode().refine(
+                        m_graph, node,
+                        arrayEdge->prediction() & SpecCell,
+                        SpecInt32Only,
+                        element->prediction()));
+            }
+            blessArrayOperation(arrayEdge, Edge(), storageEdge);
+            fixEdge<KnownCellUse>(arrayEdge);
+
+            // Convert `array.push()` to GetArrayLength.
+            if (!elementCount && node->arrayMode().supportsSelfLength()) {
+                node->setOpAndDefaultFlags(GetArrayLength);
+                node->child1() = arrayEdge;
+                node->child2() = storageEdge;
+                fixEdge<KnownCellUse>(node->child1());
+                break;
+            }
+
+            // We do not want to perform osr exit and retry for ArrayPush. We insert Check with appropriate type,
+            // and ArrayPush uses the edge as known typed edge. Therefore, ArrayPush do not need to perform type checks.
+            for (unsigned i = 0; i < elementCount; ++i) {
+                Edge& element = m_graph.varArgChild(node, i + elementOffset);
+                switch (node->arrayMode().type()) {
+                case Array::Int32:
+                    insertCheck<Int32Use>(element.node());
+                    fixEdge<KnownInt32Use>(element);
+                    break;
+                case Array::Double:
+                    insertCheck<DoubleRepRealUse>(element.node());
+                    fixEdge<DoubleRepUse>(element);
+                    break;
+                case Array::Contiguous:
+                case Array::ArrayStorage:
+                    speculateForBarrier(element);
+                    break;
+                default:
+                    break;
+                }
+                ASSERT(shouldNotHaveTypeCheck(element.useKind()));
             }
             break;