CSE DataViewGet* DFG nodes
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2018 19:27:56 +0000 (19:27 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2018 19:27:56 +0000 (19:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188768

Reviewed by Yusuke Suzuki.

JSTests:

* microbenchmarks/dataview-cse.js: Added.
(assert):
(test):
* stress/dataview-get-cse.js: Added.
(assert):
(test1.foo):
(test1):
(test2.foo):
(test2):
(test3.foo):
(test3):
(test4.foo):
(test4):
(test5.foo):
(test5):
(test6.foo):
(test6):

Source/JavaScriptCore:

This patch makes it so that we CSE DataViewGet* accesses. To do this,
I needed to add a third descriptor to HeapLocation to represent the
isLittleEndian child. This patch is neutral on compile time benchmarks,
and is a 50% speedup on a trivial CSE microbenchmark that I added.

* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGHeapLocation.cpp:
(WTF::printInternal):
* dfg/DFGHeapLocation.h:
(JSC::DFG::HeapLocation::HeapLocation):
(JSC::DFG::HeapLocation::hash const):
(JSC::DFG::HeapLocation::operator== const):
(JSC::DFG::indexedPropertyLocForResultType):

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

JSTests/ChangeLog
JSTests/microbenchmarks/dataview-cse.js [new file with mode: 0644]
JSTests/stress/dataview-get-cse.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGHeapLocation.cpp
Source/JavaScriptCore/dfg/DFGHeapLocation.h

index 686d90a..7c73321 100644 (file)
@@ -1,3 +1,28 @@
+2018-08-30  Saam barati  <sbarati@apple.com>
+
+        CSE DataViewGet* DFG nodes
+        https://bugs.webkit.org/show_bug.cgi?id=188768
+
+        Reviewed by Yusuke Suzuki.
+
+        * microbenchmarks/dataview-cse.js: Added.
+        (assert):
+        (test):
+        * stress/dataview-get-cse.js: Added.
+        (assert):
+        (test1.foo):
+        (test1):
+        (test2.foo):
+        (test2):
+        (test3.foo):
+        (test3):
+        (test4.foo):
+        (test4):
+        (test5.foo):
+        (test5):
+        (test6.foo):
+        (test6):
+
 2018-08-30  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         output of toString() of Generator is wrong
diff --git a/JSTests/microbenchmarks/dataview-cse.js b/JSTests/microbenchmarks/dataview-cse.js
new file mode 100644 (file)
index 0000000..fcdf93d
--- /dev/null
@@ -0,0 +1,28 @@
+"use strict";
+
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+function test(dv, littleEndian) {
+    return dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+    + dv.getInt32(0, littleEndian)
+}
+noInline(test);
+
+let ab = new ArrayBuffer(4);
+let dv = new DataView(ab);
+dv.setInt32(0, 10, true);
+for (let i = 0; i < 1000000; ++i) {
+    let result = test(dv, true);
+    assert(result === 10*10);
+}
diff --git a/JSTests/stress/dataview-get-cse.js b/JSTests/stress/dataview-get-cse.js
new file mode 100644 (file)
index 0000000..e3d47a9
--- /dev/null
@@ -0,0 +1,137 @@
+"use strict";
+
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+
+function test1() {
+    function foo(dv) {
+        return [dv.getFloat32(0), dv.getFloat64(0)];
+    }
+    noInline(foo);
+
+    let ab = new ArrayBuffer(8);
+    let dv = new DataView(ab);
+    dv.setFloat64(0, 128431.42342189432, false);
+    for (let i = 0; i < 10000; ++i) {
+        let result = foo(dv);
+        assert(result[0] !== result[1]);
+    }
+}
+test1();
+
+function test2() {
+    function foo(dv) {
+        return [dv.getFloat32(0), dv.getFloat32(0)];
+    }
+    noInline(foo);
+
+    let ab = new ArrayBuffer(8);
+    let dv = new DataView(ab);
+    dv.setFloat64(0, 128431.42342189432, false);
+    for (let i = 0; i < 10000; ++i) {
+        let result = foo(dv);
+        assert(result[0] === result[1]);
+    }
+}
+test2();
+
+function test3() {
+    function foo(dv, ta) {
+        let a = dv.getFloat64(0, true);
+        ta[0] = Math.PI;
+        let b = dv.getFloat64(0, true);
+        return [a, b];
+    }
+    noInline(foo);
+
+    let ab = new ArrayBuffer(8);
+    let dv = new DataView(ab);
+    let ta = new Float64Array(ab);
+    for (let i = 0; i < 40000; ++i) {
+        dv.setFloat64(0, 0.0, true);
+        let result = foo(dv, ta);
+        assert(result[0] === 0.0);
+        assert(result[1] === Math.PI);
+    }
+}
+test3();
+
+function test4() {
+    function foo(dv) {
+        let a = dv.getInt32(0, true);
+        let b = dv.getInt32(0, false);
+        return [a, b];
+    }
+    noInline(foo);
+
+    let ab = new ArrayBuffer(8);
+    let dv = new DataView(ab);
+    dv.setInt32(0, 0x11223344, true);
+    for (let i = 0; i < 40000; ++i) {
+        let result = foo(dv);
+        assert(result[0] === 0x11223344);
+        assert(result[1] === 0x44332211)
+    }
+}
+test4();
+
+function test5() {
+    function foo(dv, littleEndian) {
+        let a = dv.getInt32(0, littleEndian);
+        let b = dv.getInt32(0, !littleEndian);
+        return [a, b];
+    }
+    noInline(foo);
+
+    let ab = new ArrayBuffer(8);
+    let dv = new DataView(ab);
+    dv.setInt32(0, 0x11223344, true);
+    for (let i = 0; i < 40000; ++i) {
+        let result = foo(dv, true);
+        assert(result[0] === 0x11223344);
+        assert(result[1] === 0x44332211)
+    }
+}
+test5();
+
+function test6() {
+    function foo(dv, littleEndian) {
+        let a = dv.getInt32(0, littleEndian);
+        let b = dv.getInt32(0, littleEndian);
+        return [a, b];
+    }
+    noInline(foo);
+
+    let ab = new ArrayBuffer(8);
+    let dv = new DataView(ab);
+    dv.setInt32(0, 0x11223344, true);
+    for (let i = 0; i < 40000; ++i) {
+        let result = foo(dv, true);
+        assert(result[0] === 0x11223344);
+        assert(result[1] === 0x11223344)
+    }
+}
+test6();
+
+function test7() {
+    function foo(dv) {
+        let a = dv.getInt32(0, true);
+        let b = dv.getInt32(4, true);
+        return [a, b];
+    }
+    noInline(foo);
+
+    let ab = new ArrayBuffer(8);
+    let dv = new DataView(ab);
+    dv.setInt32(0, 0x11223344, true);
+    dv.setInt32(4, 0x12121212, true);
+    for (let i = 0; i < 40000; ++i) {
+        let result = foo(dv, true);
+        assert(result[0] === 0x11223344);
+        assert(result[1] === 0x12121212);
+    }
+}
+test7();
index bc52e6f..95c8361 100644 (file)
@@ -1,3 +1,27 @@
+2018-08-30  Saam barati  <sbarati@apple.com>
+
+        CSE DataViewGet* DFG nodes
+        https://bugs.webkit.org/show_bug.cgi?id=188768
+
+        Reviewed by Yusuke Suzuki.
+
+        This patch makes it so that we CSE DataViewGet* accesses. To do this,
+        I needed to add a third descriptor to HeapLocation to represent the
+        isLittleEndian child. This patch is neutral on compile time benchmarks,
+        and is a 50% speedup on a trivial CSE microbenchmark that I added.
+
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGHeapLocation.cpp:
+        (WTF::printInternal):
+        * dfg/DFGHeapLocation.h:
+        (JSC::DFG::HeapLocation::HeapLocation):
+        (JSC::DFG::HeapLocation::hash const):
+        (JSC::DFG::HeapLocation::operator== const):
+        (JSC::DFG::indexedPropertyLocForResultType):
+
 2018-08-30  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         output of toString() of Generator is wrong
index 804d4d4..641b1ad 100644 (file)
@@ -1763,6 +1763,9 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     case DataViewGetInt: {
         read(MiscFields);
         read(TypedArrayProperties);
+        LocationKind indexedPropertyLoc = indexedPropertyLocForResultType(node->result());
+        def(HeapLocation(indexedPropertyLoc, AbstractHeap(TypedArrayProperties, node->dataViewData().asQuadWord),
+            node->child1(), node->child2(), node->child3()), LazyNode(node));
         return;
     }
 
index fb2c06d..c0a70cd 100644 (file)
@@ -2119,7 +2119,7 @@ private:
                     node->setResult(NodeResultInt32);
                     break;
                 case 4:
-                    if (data.isSigned) 
+                    if (data.isSigned)
                         node->setResult(NodeResultInt32);
                     else
                         node->setResult(NodeResultInt52);
index 016ad0e..38e27ff 100644 (file)
@@ -132,6 +132,10 @@ void printInternal(PrintStream& out, LocationKind kind)
         out.print("IndexedPropertyDoubleSaneChainLoc");
         return;
 
+    case IndexedPropertyInt32Loc:
+        out.print("IndexedPropertyInt32Loc");
+        return;
+
     case IndexedPropertyInt52Loc:
         out.print("IndexedPropertyInt52Loc");
         return;
index 674251e..b3d2040 100644 (file)
@@ -49,6 +49,7 @@ enum LocationKind {
     HasIndexedPropertyLoc,
     IndexedPropertyDoubleLoc,
     IndexedPropertyDoubleSaneChainLoc,
+    IndexedPropertyInt32Loc,
     IndexedPropertyInt52Loc,
     IndexedPropertyJSLoc,
     IndexedPropertyStorageLoc,
@@ -77,24 +78,25 @@ public:
     HeapLocation(
         LocationKind kind = InvalidLocationKind,
         AbstractHeap heap = AbstractHeap(),
-        Node* base = nullptr, LazyNode index = LazyNode())
+        Node* base = nullptr, LazyNode index = LazyNode(), Node* descriptor = nullptr)
         : m_kind(kind)
         , m_heap(heap)
         , m_base(base)
         , m_index(index)
+        , m_descriptor(descriptor)
     {
         ASSERT((kind == InvalidLocationKind) == !heap);
         ASSERT(!!m_heap || !m_base);
-        ASSERT(m_base || !m_index);
+        ASSERT(m_base || (!m_index && !m_descriptor));
     }
 
-    HeapLocation(LocationKind kind, AbstractHeap heap, Node* base, Node* index)
-        : HeapLocation(kind, heap, base, LazyNode(index))
+    HeapLocation(LocationKind kind, AbstractHeap heap, Node* base, Node* index, Node* descriptor = nullptr)
+        : HeapLocation(kind, heap, base, LazyNode(index), descriptor)
     {
     }
     
-    HeapLocation(LocationKind kind, AbstractHeap heap, Edge base, Edge index = Edge())
-        : HeapLocation(kind, heap, base.node(), index.node())
+    HeapLocation(LocationKind kind, AbstractHeap heap, Edge base, Edge index = Edge(), Edge descriptor = Edge())
+        : HeapLocation(kind, heap, base.node(), index.node(), descriptor.node())
     {
     }
     
@@ -103,6 +105,7 @@ public:
         , m_heap(WTF::HashTableDeletedValue)
         , m_base(nullptr)
         , m_index(nullptr)
+        , m_descriptor(nullptr)
     {
     }
     
@@ -115,7 +118,7 @@ public:
     
     unsigned hash() const
     {
-        return m_kind + m_heap.hash() + m_index.hash() + m_kind;
+        return m_kind + m_heap.hash() + m_index.hash() + static_cast<unsigned>(bitwise_cast<uintptr_t>(m_base)) + static_cast<unsigned>(bitwise_cast<uintptr_t>(m_descriptor));
     }
     
     bool operator==(const HeapLocation& other) const
@@ -123,7 +126,8 @@ public:
         return m_kind == other.m_kind
             && m_heap == other.m_heap
             && m_base == other.m_base
-            && m_index == other.m_index;
+            && m_index == other.m_index
+            && m_descriptor == other.m_descriptor;
     }
     
     bool isHashTableDeletedValue() const
@@ -138,6 +142,7 @@ private:
     AbstractHeap m_heap;
     Node* m_base;
     LazyNode m_index;
+    Node* m_descriptor;
 };
 
 struct HeapLocationHash {
@@ -159,6 +164,8 @@ inline LocationKind indexedPropertyLocForResultType(NodeFlags canonicalResultRep
         return IndexedPropertyDoubleLoc;
     case NodeResultInt52:
         return IndexedPropertyInt52Loc;
+    case NodeResultInt32:
+        return IndexedPropertyInt32Loc;
     case NodeResultJS:
         return IndexedPropertyJSLoc;
     case NodeResultStorage: