[JSC] Use GetArrayLength for JSArray.length even when the array type is undecided
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Sep 2016 22:13:37 +0000 (22:13 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Sep 2016 22:13:37 +0000 (22:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161671

Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-09-12
Reviewed by Geoffrey Garen.

JSTests:

* stress/get-array-length-on-undecided.js: Added.

Source/JavaScriptCore:

UndecidedShape is a type with storage. When we allocate an uninitialized JSArray,
it gets a butterfly with its length.
When we were querying that length, we were generating a generic GetById with inline cache.

This patch adds the missing bits to treat Undecided like the other types with storage.

* dfg/DFGArrayMode.cpp:
(JSC::DFG::canBecomeGetArrayLength):
(JSC::DFG::ArrayMode::refine):
* dfg/DFGArrayMode.h:
(JSC::DFG::ArrayMode::usesButterfly):
(JSC::DFG::ArrayMode::lengthNeedsStorage):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::checkArray):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetArrayLength):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):

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

JSTests/ChangeLog
JSTests/stress/get-array-length-on-undecided.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGArrayMode.cpp
Source/JavaScriptCore/dfg/DFGArrayMode.h
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index f171005..5ea140a 100644 (file)
@@ -1,3 +1,12 @@
+2016-09-12  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Use GetArrayLength for JSArray.length even when the array type is undecided
+        https://bugs.webkit.org/show_bug.cgi?id=161671
+
+        Reviewed by Geoffrey Garen.
+
+        * stress/get-array-length-on-undecided.js: Added.
+
 2016-09-12  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [DFG][FTL] Add ArithTan
diff --git a/JSTests/stress/get-array-length-on-undecided.js b/JSTests/stress/get-array-length-on-undecided.js
new file mode 100644 (file)
index 0000000..943866d
--- /dev/null
@@ -0,0 +1,76 @@
+function forceTransition() {
+    // We want to test the StructureCheck in testSparseArray(), not this watchpoint.
+    // We start with the transition so that it's nothing new.
+    let array = new Array();
+    array[100001] = "WebKit!";
+}
+forceTransition();
+
+function opaqueGetArrayLength(array)
+{
+    return array.length;
+}
+noInline(opaqueGetArrayLength);
+
+function testEmptyArray()
+{
+    let array = [];
+    for (let i = 0; i < 1e6; ++i) {
+        if (opaqueGetArrayLength(array) !== 0) {
+            throw "Failed testEmptyArray";
+        }
+    }
+
+    array = new Array();
+    for (let i = 0; i < 1e6; ++i) {
+        if (opaqueGetArrayLength(array) !== 0) {
+            throw "Failed testEmptyArray";
+        }
+    }
+}
+testEmptyArray();
+
+
+function testUnitializedArray()
+{
+    let array = new Array(32);
+    for (let i = 0; i < 1e6; ++i) {
+        if (opaqueGetArrayLength(array) !== 32) {
+            throw "Failed testUnitializedArray";
+        }
+    }
+
+    array = new Array();
+    array.length = 64
+    for (let i = 0; i < 1e6; ++i) {
+        if (opaqueGetArrayLength(array) !== 64) {
+            throw "Failed testUnitializedArray";
+        }
+    }
+}
+testUnitializedArray();
+
+function testOversizedArray()
+{
+    let array = new Array(100001);
+    for (let i = 0; i < 1e6; ++i) {
+        if (opaqueGetArrayLength(array) !== 100001) {
+            throw "Failed testOversizedArray";
+        }
+    }
+}
+testOversizedArray();
+
+// This should OSR Exit and fallback to GetById to get the length.
+function testSparseArray()
+{
+    let array = new Array();
+    array[100001] = "WebKit!";
+    for (let i = 0; i < 1e6; ++i) {
+        if (opaqueGetArrayLength(array) !== 100002) {
+            throw "Failed testOversizedArray";
+        }
+    }
+}
+testSparseArray();
+
index 3fddd1a..6e95af2 100644 (file)
@@ -1,3 +1,33 @@
+2016-09-12  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Use GetArrayLength for JSArray.length even when the array type is undecided
+        https://bugs.webkit.org/show_bug.cgi?id=161671
+
+        Reviewed by Geoffrey Garen.
+
+        UndecidedShape is a type with storage. When we allocate an uninitialized JSArray,
+        it gets a butterfly with its length.
+        When we were querying that length, we were generating a generic GetById with inline cache.
+
+        This patch adds the missing bits to treat Undecided like the other types with storage.
+
+        * dfg/DFGArrayMode.cpp:
+        (JSC::DFG::canBecomeGetArrayLength):
+        (JSC::DFG::ArrayMode::refine):
+        * dfg/DFGArrayMode.h:
+        (JSC::DFG::ArrayMode::usesButterfly):
+        (JSC::DFG::ArrayMode::lengthNeedsStorage):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::checkArray):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetArrayLength):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
+
 2016-09-12  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [DFG][FTL] Add ArithTan
index 4ea7512..56a2a65 100644 (file)
@@ -150,6 +150,14 @@ ArrayMode ArrayMode::fromObserved(const ConcurrentJITLocker& locker, ArrayProfil
     }
 }
 
+static bool canBecomeGetArrayLength(Graph& graph, Node* node)
+{
+    if (node->op() != GetById)
+        return false;
+    auto uid = graph.identifiers()[node->identifierNumber()];
+    return uid == graph.m_vm.propertyNames->length.impl();
+}
+
 ArrayMode ArrayMode::refine(
     Graph& graph, Node* node,
     SpeculatedType base, SpeculatedType index, SpeculatedType value) const
@@ -192,7 +200,7 @@ ArrayMode ArrayMode::refine(
         // If we have an OriginalArray and the JSArray prototype chain is sane,
         // any indexed access always return undefined. We have a fast path for that.
         JSGlobalObject* globalObject = graph.globalObjectFor(node->origin.semantic);
-        if (node->op() == GetByVal
+        if ((node->op() == GetByVal || canBecomeGetArrayLength(graph, node))
             && arrayClass() == Array::OriginalArray
             && globalObject->arrayPrototypeChainIsSane()
             && !graph.hasExitSite(node->origin.semantic, OutOfBounds)) {
index 749ce2b..2ec4803 100644 (file)
@@ -234,6 +234,7 @@ public:
     bool usesButterfly() const
     {
         switch (type()) {
+        case Array::Undecided:
         case Array::Int32:
         case Array::Double:
         case Array::Contiguous:
@@ -312,6 +313,7 @@ public:
     bool lengthNeedsStorage() const
     {
         switch (type()) {
+        case Array::Undecided:
         case Array::Int32:
         case Array::Double:
         case Array::Contiguous:
index 40a0740..55db094 100644 (file)
@@ -942,6 +942,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     case GetArrayLength: {
         ArrayMode mode = node->arrayMode();
         switch (mode.type()) {
+        case Array::Undecided:
         case Array::Int32:
         case Array::Double:
         case Array::Contiguous:
index d3211c5..f53989d 100644 (file)
@@ -2102,7 +2102,7 @@ private:
         }
         
         if (!storageCheck(arrayMode))
-            return 0;
+            return nullptr;
         
         if (arrayMode.usesButterfly()) {
             return m_insertionSet.insertNode(
index d052089..6f172c7 100644 (file)
@@ -6051,6 +6051,7 @@ void SpeculativeJIT::compileGetGlobalObject(Node* node)
 void SpeculativeJIT::compileGetArrayLength(Node* node)
 {
     switch (node->arrayMode().type()) {
+    case Array::Undecided:
     case Array::Int32:
     case Array::Double:
     case Array::Contiguous: {
index 50f6c0a..e61b4ef 100644 (file)
@@ -288,6 +288,7 @@ inline CapabilityLevel canCompile(Node* node)
         break;
     case GetArrayLength:
         switch (node->arrayMode().type()) {
+        case Array::Undecided:
         case Array::Int32:
         case Array::Double:
         case Array::Contiguous:
index 42d185a..0b1a02f 100644 (file)
@@ -2900,6 +2900,7 @@ private:
     void compileGetArrayLength()
     {
         switch (m_node->arrayMode().type()) {
+        case Array::Undecided:
         case Array::Int32:
         case Array::Double:
         case Array::Contiguous: {