[FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Feb 2018 06:41:50 +0000 (06:41 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Feb 2018 06:41:50 +0000 (06:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182792

Reviewed by Mark Lam.

JSTests:

* stress/has-indexed-property-array-storage.js: Added.
(shouldBe):
(test1):
(test2):
* stress/has-indexed-property-slow-put-array-storage.js: Added.
(shouldBe):
(test1):
(test2):

Source/JavaScriptCore:

This patch adds HasIndexedProperty for ArrayStorage and SlowPutArrayStorage in FTL.
HasIndexedProperty with ArrayStorage frequently causes FTL compilation failures
in web-tooling-benchmarks.

* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):

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

JSTests/ChangeLog
JSTests/stress/has-indexed-property-array-storage.js [new file with mode: 0644]
JSTests/stress/has-indexed-property-slow-put-array-storage.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index 725a75f..365c84d 100644 (file)
@@ -1,3 +1,19 @@
+2018-02-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage
+        https://bugs.webkit.org/show_bug.cgi?id=182792
+
+        Reviewed by Mark Lam.
+
+        * stress/has-indexed-property-array-storage.js: Added.
+        (shouldBe):
+        (test1):
+        (test2):
+        * stress/has-indexed-property-slow-put-array-storage.js: Added.
+        (shouldBe):
+        (test1):
+        (test2):
+
 2018-02-20  Saam Barati  <sbarati@apple.com>
 
         DFG::VarargsForwardingPhase should eliminate getting argument length
diff --git a/JSTests/stress/has-indexed-property-array-storage.js b/JSTests/stress/has-indexed-property-array-storage.js
new file mode 100644 (file)
index 0000000..276527a
--- /dev/null
@@ -0,0 +1,38 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test1(array)
+{
+    return 2 in array;
+}
+noInline(test1);
+
+var array = [1, 2, 3, 4];
+ensureArrayStorage(array);
+for (var i = 0; i < 1e5; ++i)
+    shouldBe(test1(array), true);
+
+var array = [1, 2, , 4];
+ensureArrayStorage(array);
+shouldBe(test1(array), false);
+
+var array = [];
+ensureArrayStorage(array);
+shouldBe(test1(array), false);
+
+function test2(array)
+{
+    return 2 in array;
+}
+noInline(test2);
+
+var array1 = [1, 2, 3, 4];
+ensureArrayStorage(array1);
+var array2 = [1, 2];
+ensureArrayStorage(array2);
+for (var i = 0; i < 1e5; ++i)
+    shouldBe(test2(array2), false);
+shouldBe(test2(array2), false);
+shouldBe(test2(array1), true);
diff --git a/JSTests/stress/has-indexed-property-slow-put-array-storage.js b/JSTests/stress/has-indexed-property-slow-put-array-storage.js
new file mode 100644 (file)
index 0000000..1ae4a92
--- /dev/null
@@ -0,0 +1,51 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test1(array)
+{
+    return 2 in array;
+}
+noInline(test1);
+
+var object = { a: 10 };
+Object.defineProperties(object, {
+    "0": {
+        get: function() { return this.a; },
+        set: function(x) { this.a = x; },
+    },
+});
+
+var array = [1, 2, 3, 4];
+array.__proto__ = object;
+ensureArrayStorage(array);
+for (var i = 0; i < 1e5; ++i)
+    shouldBe(test1(array), true);
+
+var array = [1, 2, , 4];
+array.__proto__ = object;
+ensureArrayStorage(array);
+shouldBe(test1(array), false);
+
+var array = [];
+array.__proto__ = object;
+ensureArrayStorage(array);
+shouldBe(test1(array), false);
+
+function test2(array)
+{
+    return 2 in array;
+}
+noInline(test2);
+
+var array1 = [1, 2, 3, 4];
+array1.__proto__ = object;
+ensureArrayStorage(array1);
+var array2 = [1, 2];
+array2.__proto__ = object;
+ensureArrayStorage(array2);
+for (var i = 0; i < 1e5; ++i)
+    shouldBe(test2(array2), false);
+shouldBe(test2(array2), false);
+shouldBe(test2(array1), true);
index 11f3c13..3406289 100644 (file)
@@ -1,3 +1,19 @@
+2018-02-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage
+        https://bugs.webkit.org/show_bug.cgi?id=182792
+
+        Reviewed by Mark Lam.
+
+        This patch adds HasIndexedProperty for ArrayStorage and SlowPutArrayStorage in FTL.
+        HasIndexedProperty with ArrayStorage frequently causes FTL compilation failures
+        in web-tooling-benchmarks.
+
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
+
 2018-02-22  Mark Lam  <mark.lam@apple.com>
 
         Refactor MacroAssembler code to improve reuse and extensibility.
index 418d902..81730f9 100644 (file)
@@ -237,6 +237,7 @@ inline CapabilityLevel canCompile(Node* node)
     case BooleanToNumber:
     case HasGenericProperty:
     case HasStructureProperty:
+    case HasIndexedProperty:
     case GetDirectPname:
     case GetEnumerableLength:
     case GetIndexedPropertyStorage:
@@ -387,17 +388,6 @@ inline CapabilityLevel canCompile(Node* node)
             RELEASE_ASSERT_NOT_REACHED();
         }
         break;
-    case HasIndexedProperty:
-        switch (node->arrayMode().type()) {
-        case Array::ForceExit:
-        case Array::Int32:
-        case Array::Double:
-        case Array::Contiguous:
-            break;
-        default:
-            return CannotCompile;
-        }
-        break;
     case GetByVal:
         switch (node->arrayMode().type()) {
         case Array::ForceExit:
index 14803ec..10c47d1 100644 (file)
@@ -9839,19 +9839,20 @@ private:
             IndexedAbstractHeap& heap = m_node->arrayMode().type() == Array::Int32 ?
                 m_heaps.indexedInt32Properties : m_heaps.indexedContiguousProperties;
 
-            LBasicBlock checkHole = m_out.newBlock();
             LBasicBlock slowCase = m_out.newBlock();
             LBasicBlock continuation = m_out.newBlock();
+            LBasicBlock lastNext = nullptr;
 
             if (!m_node->arrayMode().isInBounds()) {
+                LBasicBlock checkHole = m_out.newBlock();
                 m_out.branch(
                     m_out.aboveOrEqual(
                         index, m_out.load32NonNegative(storage, m_heaps.Butterfly_publicLength)),
                     rarely(slowCase), usually(checkHole));
+                lastNext = m_out.appendTo(checkHole, slowCase);
             } else
-                m_out.jump(checkHole);
+                lastNext = m_out.insertNewBlocksBefore(slowCase);
 
-            LBasicBlock lastNext = m_out.appendTo(checkHole, slowCase);
             LValue checkHoleResultValue =
                 m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_node->child2())));
             ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
@@ -9875,19 +9876,20 @@ private:
             
             IndexedAbstractHeap& heap = m_heaps.indexedDoubleProperties;
             
-            LBasicBlock checkHole = m_out.newBlock();
             LBasicBlock slowCase = m_out.newBlock();
             LBasicBlock continuation = m_out.newBlock();
+            LBasicBlock lastNext = nullptr;
             
             if (!m_node->arrayMode().isInBounds()) {
+                LBasicBlock checkHole = m_out.newBlock();
                 m_out.branch(
                     m_out.aboveOrEqual(
                         index, m_out.load32NonNegative(storage, m_heaps.Butterfly_publicLength)),
                     rarely(slowCase), usually(checkHole));
+                lastNext = m_out.appendTo(checkHole, slowCase);
             } else
-                m_out.jump(checkHole);
+                lastNext = m_out.insertNewBlocksBefore(slowCase);
 
-            LBasicBlock lastNext = m_out.appendTo(checkHole, slowCase);
             LValue doubleValue = m_out.loadDouble(baseIndex(heap, storage, index, m_node->child2()));
             LValue checkHoleResultValue = m_out.doubleEqual(doubleValue, doubleValue);
             ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
@@ -9903,10 +9905,52 @@ private:
             setBoolean(m_out.phi(Int32, checkHoleResult, slowResult));
             return;
         }
-            
-        default:
-            RELEASE_ASSERT_NOT_REACHED();
-            return;
+
+        case Array::ArrayStorage: {
+            LValue base = lowCell(m_node->child1());
+            LValue index = lowInt32(m_node->child2());
+            LValue storage = lowStorage(m_node->child3());
+            LValue internalMethodType = m_out.constInt32(static_cast<int32_t>(m_node->internalMethodType()));
+
+            LBasicBlock slowCase = m_out.newBlock();
+            LBasicBlock continuation = m_out.newBlock();
+            LBasicBlock lastNext = nullptr;
+
+            if (!m_node->arrayMode().isInBounds()) {
+                LBasicBlock checkHole = m_out.newBlock();
+                m_out.branch(
+                    m_out.aboveOrEqual(
+                        index, m_out.load32NonNegative(storage, m_heaps.Butterfly_vectorLength)),
+                    rarely(slowCase), usually(checkHole));
+                lastNext = m_out.appendTo(checkHole, slowCase);
+            } else
+                lastNext = m_out.insertNewBlocksBefore(slowCase);
+
+            LValue checkHoleResultValue =
+                m_out.notZero64(m_out.load64(baseIndex(m_heaps.ArrayStorage_vector, storage, index, m_node->child2())));
+            ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
+            m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+
+            m_out.appendTo(slowCase, continuation);
+            ValueFromBlock slowResult = m_out.anchor(m_out.equal(
+                m_out.constInt64(JSValue::encode(jsBoolean(true))),
+                vmCall(Int64, m_out.operation(operationHasIndexedPropertyByInt), m_callFrame, base, index, internalMethodType)));
+            m_out.jump(continuation);
+
+            m_out.appendTo(continuation, lastNext);
+            setBoolean(m_out.phi(Int32, checkHoleResult, slowResult));
+            break;
+        }
+
+        default: {
+            LValue base = lowCell(m_node->child1());
+            LValue index = lowInt32(m_node->child2());
+            LValue internalMethodType = m_out.constInt32(static_cast<int32_t>(m_node->internalMethodType()));
+            setBoolean(m_out.equal(
+                m_out.constInt64(JSValue::encode(jsBoolean(true))),
+                vmCall(Int64, m_out.operation(operationHasIndexedPropertyByInt), m_callFrame, base, index, internalMethodType)));
+            break;
+        }
         }
     }