Title: [228943] trunk
Revision
228943
Author
utatane....@gmail.com
Date
2018-02-22 22:41:50 -0800 (Thu, 22 Feb 2018)

Log Message

[FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage
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/_javascript_Core:

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):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (228942 => 228943)


--- trunk/JSTests/ChangeLog	2018-02-23 04:18:17 UTC (rev 228942)
+++ trunk/JSTests/ChangeLog	2018-02-23 06:41:50 UTC (rev 228943)
@@ -1,3 +1,19 @@
+2018-02-22  Yusuke Suzuki  <utatane....@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  <sbar...@apple.com>
 
         DFG::VarargsForwardingPhase should eliminate getting argument length

Added: trunk/JSTests/stress/has-indexed-property-array-storage.js (0 => 228943)


--- trunk/JSTests/stress/has-indexed-property-array-storage.js	                        (rev 0)
+++ trunk/JSTests/stress/has-indexed-property-array-storage.js	2018-02-23 06:41:50 UTC (rev 228943)
@@ -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);

Added: trunk/JSTests/stress/has-indexed-property-slow-put-array-storage.js (0 => 228943)


--- trunk/JSTests/stress/has-indexed-property-slow-put-array-storage.js	                        (rev 0)
+++ trunk/JSTests/stress/has-indexed-property-slow-put-array-storage.js	2018-02-23 06:41:50 UTC (rev 228943)
@@ -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);

Modified: trunk/Source/_javascript_Core/ChangeLog (228942 => 228943)


--- trunk/Source/_javascript_Core/ChangeLog	2018-02-23 04:18:17 UTC (rev 228942)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-02-23 06:41:50 UTC (rev 228943)
@@ -1,3 +1,19 @@
+2018-02-22  Yusuke Suzuki  <utatane....@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....@apple.com>
 
         Refactor MacroAssembler code to improve reuse and extensibility.

Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (228942 => 228943)


--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2018-02-23 04:18:17 UTC (rev 228942)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2018-02-23 06:41:50 UTC (rev 228943)
@@ -237,6 +237,7 @@
     case BooleanToNumber:
     case HasGenericProperty:
     case HasStructureProperty:
+    case HasIndexedProperty:
     case GetDirectPname:
     case GetEnumerableLength:
     case GetIndexedPropertyStorage:
@@ -387,17 +388,6 @@
             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:

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (228942 => 228943)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-02-23 04:18:17 UTC (rev 228942)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-02-23 06:41:50 UTC (rev 228943)
@@ -9839,19 +9839,20 @@
             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 @@
             
             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,11 +9905,53 @@
             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;
+        }
+        }
     }
 
     void compileHasGenericProperty()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to