Title: [228726] trunk
Revision
228726
Author
utatane....@gmail.com
Date
2018-02-19 20:00:16 -0800 (Mon, 19 Feb 2018)

Log Message

[FTL] Add Arrayify for ArrayStorage and SlowPutArrayStorage
https://bugs.webkit.org/show_bug.cgi?id=182731

Reviewed by Saam Barati.

JSTests:

* stress/arrayify-array-storage-array.js: Added.
(shouldBe):
(testArrayStorage):
* stress/arrayify-array-storage-non-array.js: Added.
(shouldBe):
(testArrayStorage):
* stress/arrayify-array-storage.js: Added.
(shouldBe):
(testArrayStorage):
* stress/arrayify-slow-put-array-storage-pass-array-storage.js: Added.
(shouldBe):
(testArrayStorage):
* stress/arrayify-slow-put-array-storage.js: Added.
(shouldBe):
(testArrayStorage):

Source/_javascript_Core:

This patch adds support for Arrayify(ArrayStorage/SlowPutArrayStorage) to FTL.
Due to ArrayifyToStructure and CheckArray changes, necessary changes for
supporting Arrayify in FTL are already done. Just allowing it in FTLCapabilities.cpp
is enough.

We fix FTL's CheckArray logic. Previously, CheckArray(SlowPutArrayStorage) does not pass
ArrayStorage in FTL. But now it passes this as DFG does. Moreover, we fix DFG's CheckArray
where CheckArray(ArrayStorage+NonArray) can pass ArrayStorage+Array.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::silentFill):
(JSC::DFG::SpeculativeJIT::jumpSlowForUnwantedArrayMode):
* dfg/DFGSpeculativeJIT.h:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::isArrayTypeForArrayify):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (228725 => 228726)


--- trunk/JSTests/ChangeLog	2018-02-20 03:45:03 UTC (rev 228725)
+++ trunk/JSTests/ChangeLog	2018-02-20 04:00:16 UTC (rev 228726)
@@ -1,3 +1,26 @@
+2018-02-14  Yusuke Suzuki  <utatane....@gmail.com>
+
+        [FTL] Add Arrayify for ArrayStorage and SlowPutArrayStorage
+        https://bugs.webkit.org/show_bug.cgi?id=182731
+
+        Reviewed by Saam Barati.
+
+        * stress/arrayify-array-storage-array.js: Added.
+        (shouldBe):
+        (testArrayStorage):
+        * stress/arrayify-array-storage-non-array.js: Added.
+        (shouldBe):
+        (testArrayStorage):
+        * stress/arrayify-array-storage.js: Added.
+        (shouldBe):
+        (testArrayStorage):
+        * stress/arrayify-slow-put-array-storage-pass-array-storage.js: Added.
+        (shouldBe):
+        (testArrayStorage):
+        * stress/arrayify-slow-put-array-storage.js: Added.
+        (shouldBe):
+        (testArrayStorage):
+
 2018-02-19  Saam Barati  <sbar...@apple.com>
 
         Don't use JSFunction's allocation profile when getting the prototype can be effectful

Added: trunk/JSTests/stress/arrayify-array-storage-array.js (0 => 228726)


--- trunk/JSTests/stress/arrayify-array-storage-array.js	                        (rev 0)
+++ trunk/JSTests/stress/arrayify-array-storage-array.js	2018-02-20 04:00:16 UTC (rev 228726)
@@ -0,0 +1,29 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var array = [1, 2, 3, 4, 5];
+var array2 = [1, "HELLO", 3, 4, 5];
+var array3 = [0.1, "OK", 0.3, 0.4, 0.5];
+ensureArrayStorage(array2);
+array.ok = 42;
+array2.ok = 42;
+array3.ok = 42;
+
+// Arrayify(ArrayStorage) works with ftl-eager
+function testArrayStorage(array)
+{
+    return array.length;
+}
+noInline(testArrayStorage);
+
+for (var i = 0; i < 1e6; ++i) {
+    shouldBe(testArrayStorage(array), 5);
+    shouldBe(testArrayStorage(array2), 5);
+    shouldBe(testArrayStorage(array3), 5);
+}
+
+var array4 = {0:1, 1:"HELLO", 2:3, 3:4, 4:5};
+ensureArrayStorage(array4);
+shouldBe(testArrayStorage(array4), undefined);

Added: trunk/JSTests/stress/arrayify-array-storage-non-array.js (0 => 228726)


--- trunk/JSTests/stress/arrayify-array-storage-non-array.js	                        (rev 0)
+++ trunk/JSTests/stress/arrayify-array-storage-non-array.js	2018-02-20 04:00:16 UTC (rev 228726)
@@ -0,0 +1,28 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var array = {0:1, 1:2, 2:3, 3:4, 4:5};
+var array2 = {0:1, 1:"HELLO", 2:3, 3:4, 4:5};
+var array3 = {0:0.1, 1:"OK", 2:0.3, 3:0.4, 4:0.5};
+ensureArrayStorage(array2);
+array.ok = 42;
+array2.ok = 42;
+array3.ok = 42;
+
+// Arrayify(ArrayStorage) works with ftl-eager
+function testArrayStorage(array)
+{
+    return array[4];
+}
+noInline(testArrayStorage);
+
+for (var i = 0; i < 1e6; ++i) {
+    shouldBe(testArrayStorage(array), 5);
+    shouldBe(testArrayStorage(array2), 5);
+    shouldBe(testArrayStorage(array3), 0.5);
+}
+
+var array4 = [0, 1, 2, 3, 4];
+shouldBe(testArrayStorage(array4), 4);

Added: trunk/JSTests/stress/arrayify-array-storage.js (0 => 228726)


--- trunk/JSTests/stress/arrayify-array-storage.js	                        (rev 0)
+++ trunk/JSTests/stress/arrayify-array-storage.js	2018-02-20 04:00:16 UTC (rev 228726)
@@ -0,0 +1,25 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var array = [1, 2, 3, 4, 5];
+var array2 = [1, "HELLO", 3, 4, 5];
+var array3 = [0.1, "OK", 0.3, 0.4, 0.5];
+ensureArrayStorage(array2);
+array.ok = 42;
+array2.ok = 42;
+array3.ok = 42;
+
+// Arrayify(ArrayStorage) works with ftl-eager
+function testArrayStorage(array)
+{
+    return array.length;
+}
+noInline(testArrayStorage);
+
+for (var i = 0; i < 1e6; ++i) {
+    shouldBe(testArrayStorage(array), 5);
+    shouldBe(testArrayStorage(array2), 5);
+    shouldBe(testArrayStorage(array3), 5);
+}

Added: trunk/JSTests/stress/arrayify-slow-put-array-storage-pass-array-storage.js (0 => 228726)


--- trunk/JSTests/stress/arrayify-slow-put-array-storage-pass-array-storage.js	                        (rev 0)
+++ trunk/JSTests/stress/arrayify-slow-put-array-storage-pass-array-storage.js	2018-02-20 04:00:16 UTC (rev 228726)
@@ -0,0 +1,40 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var object = { a: 10 };
+Object.defineProperties(object, {
+    "0": {
+        get: function() { return this.a; },
+        set: function(x) { this.a = x; },
+    },
+});
+
+var array1 = [0.1, "OK", 0.3, 0.4, 0.5];
+var array2 = [1, "HELLO", 3, 4, 5];
+ensureArrayStorage(array2);
+array1.ok = 42;
+array2.ok = 42;
+array2.__proto__ = object;
+
+// Arrayify(SlowPutArrayStorage) works with ftl-eager
+function testArrayStorage(array)
+{
+    return array.length;
+}
+noInline(testArrayStorage);
+
+for (var i = 0; i < 1e6; ++i) {
+    shouldBe(testArrayStorage(array1), 5);
+    shouldBe(testArrayStorage(array2), 5);
+}
+
+var array3 = [1, 2, 3];
+ensureArrayStorage(array3);
+shouldBe(testArrayStorage(array3), 3);
+var array4 = [1, 2, 3];
+shouldBe(testArrayStorage(array4), 3);
+var array5 = {0:1, 1:2};
+ensureArrayStorage(array5);
+shouldBe(testArrayStorage(array5), undefined);

Added: trunk/JSTests/stress/arrayify-slow-put-array-storage.js (0 => 228726)


--- trunk/JSTests/stress/arrayify-slow-put-array-storage.js	                        (rev 0)
+++ trunk/JSTests/stress/arrayify-slow-put-array-storage.js	2018-02-20 04:00:16 UTC (rev 228726)
@@ -0,0 +1,31 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var object = { a: 10 };
+Object.defineProperties(object, {
+    "0": {
+        get: function() { return this.a; },
+        set: function(x) { this.a = x; },
+    },
+});
+
+var array1 = [0.1, "OK", 0.3, 0.4, 0.5];
+var array2 = [1, "HELLO", 3, 4, 5];
+ensureArrayStorage(array2);
+array1.ok = 42;
+array2.ok = 42;
+array2.__proto__ = object;
+
+// Arrayify(SlowPutArrayStorage) works with ftl-eager
+function testArrayStorage(array)
+{
+    return array.length;
+}
+noInline(testArrayStorage);
+
+for (var i = 0; i < 1e6; ++i) {
+    shouldBe(testArrayStorage(array1), 5);
+    shouldBe(testArrayStorage(array2), 5);
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (228725 => 228726)


--- trunk/Source/_javascript_Core/ChangeLog	2018-02-20 03:45:03 UTC (rev 228725)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-02-20 04:00:16 UTC (rev 228726)
@@ -1,3 +1,28 @@
+2018-02-14  Yusuke Suzuki  <utatane....@gmail.com>
+
+        [FTL] Add Arrayify for ArrayStorage and SlowPutArrayStorage
+        https://bugs.webkit.org/show_bug.cgi?id=182731
+
+        Reviewed by Saam Barati.
+
+        This patch adds support for Arrayify(ArrayStorage/SlowPutArrayStorage) to FTL.
+        Due to ArrayifyToStructure and CheckArray changes, necessary changes for
+        supporting Arrayify in FTL are already done. Just allowing it in FTLCapabilities.cpp
+        is enough.
+
+        We fix FTL's CheckArray logic. Previously, CheckArray(SlowPutArrayStorage) does not pass
+        ArrayStorage in FTL. But now it passes this as DFG does. Moreover, we fix DFG's CheckArray
+        where CheckArray(ArrayStorage+NonArray) can pass ArrayStorage+Array.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::silentFill):
+        (JSC::DFG::SpeculativeJIT::jumpSlowForUnwantedArrayMode):
+        * dfg/DFGSpeculativeJIT.h:
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::isArrayTypeForArrayify):
+
 2018-02-19  Saam Barati  <sbar...@apple.com>
 
         Don't use JSFunction's allocation profile when getting the prototype can be effectful

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (228725 => 228726)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2018-02-20 03:45:03 UTC (rev 228725)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2018-02-20 04:00:16 UTC (rev 228726)
@@ -723,37 +723,6 @@
         RELEASE_ASSERT_NOT_REACHED();
     }
 }
-    
-JITCompiler::Jump SpeculativeJIT::jumpSlowForUnwantedArrayMode(GPRReg tempGPR, ArrayMode arrayMode, IndexingType shape)
-{
-    switch (arrayMode.arrayClass()) {
-    case Array::OriginalArray: {
-        CRASH();
-#if COMPILER_QUIRK(CONSIDERS_UNREACHABLE_CODE)
-        JITCompiler::Jump result; // I already know that VC++ takes unkindly to the _expression_ "return Jump()", so I'm doing it this way in anticipation of someone eventually using VC++ to compile the DFG.
-        return result;
-#endif
-    }
-        
-    case Array::Array:
-        m_jit.and32(TrustedImm32(IsArray | IndexingShapeMask), tempGPR);
-        return m_jit.branch32(
-            MacroAssembler::NotEqual, tempGPR, TrustedImm32(IsArray | shape));
-        
-    case Array::NonArray:
-    case Array::OriginalNonArray:
-        m_jit.and32(TrustedImm32(IsArray | IndexingShapeMask), tempGPR);
-        return m_jit.branch32(
-            MacroAssembler::NotEqual, tempGPR, TrustedImm32(shape));
-        
-    case Array::PossiblyArray:
-        m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
-        return m_jit.branch32(MacroAssembler::NotEqual, tempGPR, TrustedImm32(shape));
-    }
-    
-    RELEASE_ASSERT_NOT_REACHED();
-    return JITCompiler::Jump();
-}
 
 JITCompiler::JumpList SpeculativeJIT::jumpSlowForUnwantedArrayMode(GPRReg tempGPR, ArrayMode arrayMode)
 {
@@ -764,42 +733,68 @@
     case Array::Double:
     case Array::Contiguous:
     case Array::Undecided:
-        return jumpSlowForUnwantedArrayMode(tempGPR, arrayMode, arrayMode.shapeMask());
+    case Array::ArrayStorage: {
+        IndexingType shape = arrayMode.shapeMask();
+        switch (arrayMode.arrayClass()) {
+        case Array::OriginalArray:
+            RELEASE_ASSERT_NOT_REACHED();
+            return result;
 
-    case Array::ArrayStorage:
+        case Array::Array:
+            m_jit.and32(TrustedImm32(IsArray | IndexingShapeMask), tempGPR);
+            result.append(m_jit.branch32(
+                MacroAssembler::NotEqual, tempGPR, TrustedImm32(IsArray | shape)));
+            return result;
+
+        case Array::NonArray:
+        case Array::OriginalNonArray:
+            m_jit.and32(TrustedImm32(IsArray | IndexingShapeMask), tempGPR);
+            result.append(m_jit.branch32(
+                MacroAssembler::NotEqual, tempGPR, TrustedImm32(shape)));
+            return result;
+
+        case Array::PossiblyArray:
+            m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
+            result.append(m_jit.branch32(MacroAssembler::NotEqual, tempGPR, TrustedImm32(shape)));
+            return result;
+        }
+
+        RELEASE_ASSERT_NOT_REACHED();
+        return result;
+    }
+
     case Array::SlowPutArrayStorage: {
         ASSERT(!arrayMode.isJSArrayWithOriginalStructure());
-        
-        if (arrayMode.isJSArray()) {
-            if (arrayMode.isSlowPut()) {
-                result.append(
-                    m_jit.branchTest32(
-                        MacroAssembler::Zero, tempGPR, MacroAssembler::TrustedImm32(IsArray)));
-                m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
-                m_jit.sub32(TrustedImm32(ArrayStorageShape), tempGPR);
-                result.append(
-                    m_jit.branch32(
-                        MacroAssembler::Above, tempGPR,
-                        TrustedImm32(SlowPutArrayStorageShape - ArrayStorageShape)));
-                break;
-            }
-            m_jit.and32(TrustedImm32(IsArray | IndexingShapeMask), tempGPR);
+
+        switch (arrayMode.arrayClass()) {
+        case Array::OriginalArray:
+            RELEASE_ASSERT_NOT_REACHED();
+            return result;
+
+        case Array::Array:
             result.append(
-                m_jit.branch32(MacroAssembler::NotEqual, tempGPR, TrustedImm32(IsArray | ArrayStorageShape)));
+                m_jit.branchTest32(
+                    MacroAssembler::Zero, tempGPR, MacroAssembler::TrustedImm32(IsArray)));
             break;
-        }
-        m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
-        if (arrayMode.isSlowPut()) {
-            m_jit.sub32(TrustedImm32(ArrayStorageShape), tempGPR);
+
+        case Array::NonArray:
+        case Array::OriginalNonArray:
             result.append(
-                m_jit.branch32(
-                    MacroAssembler::Above, tempGPR,
-                    TrustedImm32(SlowPutArrayStorageShape - ArrayStorageShape)));
+                m_jit.branchTest32(
+                    MacroAssembler::NonZero, tempGPR, MacroAssembler::TrustedImm32(IsArray)));
             break;
+
+        case Array::PossiblyArray:
+            break;
         }
+
+        m_jit.and32(TrustedImm32(IndexingShapeMask), tempGPR);
+        m_jit.sub32(TrustedImm32(ArrayStorageShape), tempGPR);
         result.append(
-            m_jit.branch32(MacroAssembler::NotEqual, tempGPR, TrustedImm32(ArrayStorageShape)));
-        break;
+            m_jit.branch32(
+                MacroAssembler::Above, tempGPR,
+                TrustedImm32(SlowPutArrayStorageShape - ArrayStorageShape)));
+        return result;
     }
     default:
         CRASH();

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (228725 => 228726)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2018-02-20 03:45:03 UTC (rev 228725)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2018-02-20 04:00:16 UTC (rev 228726)
@@ -3306,7 +3306,6 @@
     void speculateMisc(Edge);
     void speculate(Node*, Edge);
     
-    JITCompiler::Jump jumpSlowForUnwantedArrayMode(GPRReg tempWithIndexingTypeReg, ArrayMode, IndexingType);
     JITCompiler::JumpList jumpSlowForUnwantedArrayMode(GPRReg tempWithIndexingTypeReg, ArrayMode);
     void checkArray(Node*);
     void arrayify(Node*, GPRReg baseReg, GPRReg propertyReg);

Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (228725 => 228726)


--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2018-02-20 03:45:03 UTC (rev 228725)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2018-02-20 04:00:16 UTC (rev 228726)
@@ -67,6 +67,7 @@
     case CheckStructure:
     case CheckStructureOrEmpty:
     case DoubleAsInt32:
+    case Arrayify:
     case ArrayifyToStructure:
     case PutStructure:
     case GetButterfly:
@@ -340,16 +341,6 @@
         // case because it would prevent us from catching bugs where the FTL backend
         // pipeline failed to optimize out an Identity.
         break;
-    case Arrayify:
-        switch (node->arrayMode().type()) {
-        case Array::Int32:
-        case Array::Double:
-        case Array::Contiguous:
-            break;
-        default:
-            return CannotCompile;
-        }
-        break;
     case CheckArray:
         switch (node->arrayMode().type()) {
         case Array::Int32:

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (228725 => 228726)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-02-20 03:45:03 UTC (rev 228725)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-02-20 04:00:16 UTC (rev 228726)
@@ -14550,8 +14550,7 @@
         case Array::Double:
         case Array::Contiguous:
         case Array::Undecided:
-        case Array::ArrayStorage:
-        case Array::SlowPutArrayStorage: {
+        case Array::ArrayStorage: {
             IndexingType shape = arrayMode.shapeMask();
             LValue indexingType = m_out.load8ZeroExt32(cell, m_heaps.JSCell_indexingTypeAndMisc);
 
@@ -14558,7 +14557,7 @@
             switch (arrayMode.arrayClass()) {
             case Array::OriginalArray:
                 DFG_CRASH(m_graph, m_node, "Unexpected original array");
-                return 0;
+                return nullptr;
 
             case Array::Array:
                 return m_out.equal(
@@ -14579,6 +14578,54 @@
             break;
         }
 
+        case Array::SlowPutArrayStorage: {
+            ASSERT(!arrayMode.isJSArrayWithOriginalStructure());
+            LValue indexingType = m_out.load8ZeroExt32(cell, m_heaps.JSCell_indexingTypeAndMisc);
+
+            LBasicBlock trueCase = m_out.newBlock();
+            LBasicBlock checkCase = m_out.newBlock();
+            LBasicBlock continuation = m_out.newBlock();
+
+            ValueFromBlock falseValue = m_out.anchor(m_out.booleanFalse);
+            LValue isAnArrayStorageShape = m_out.belowOrEqual(
+                m_out.sub(
+                    m_out.bitAnd(indexingType, m_out.constInt32(IndexingShapeMask)),
+                    m_out.constInt32(ArrayStorageShape)),
+                m_out.constInt32(SlowPutArrayStorageShape - ArrayStorageShape));
+            m_out.branch(isAnArrayStorageShape, usually(checkCase), usually(continuation));
+
+            LBasicBlock lastNext = m_out.appendTo(checkCase, trueCase);
+            switch (arrayMode.arrayClass()) {
+            case Array::OriginalArray:
+                DFG_CRASH(m_graph, m_node, "Unexpected original array");
+                return nullptr;
+
+            case Array::Array:
+                m_out.branch(
+                    m_out.testNonZero32(indexingType, m_out.constInt32(IsArray)),
+                    usually(trueCase), usually(continuation));
+                break;
+
+            case Array::NonArray:
+            case Array::OriginalNonArray:
+                m_out.branch(
+                    m_out.testIsZero32(indexingType, m_out.constInt32(IsArray)),
+                    usually(trueCase), usually(continuation));
+                break;
+
+            case Array::PossiblyArray:
+                m_out.jump(trueCase);
+                break;
+            }
+
+            m_out.appendTo(trueCase, continuation);
+            ValueFromBlock trueValue = m_out.anchor(m_out.booleanTrue);
+            m_out.jump(continuation);
+
+            m_out.appendTo(continuation, lastNext);
+            return m_out.phi(Int32, falseValue, trueValue);
+        }
+
         default:
             break;
         }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to