Attachment 348441 Details for Bug 188768 – patch

[patch] patch

b-backup.diff (text/plain), 11.51 KB, created by Saam Barati on 2018-08-29 15:48:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-08-29 15:48:55 PDT

Size: 11.51 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 235479)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,28 @@
>+2018-08-29  Saam barati  <sbarati@apple.com>
>+
>+        CSE DataViewGet* DFG nodes
>+        https://bugs.webkit.org/show_bug.cgi?id=188768
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * 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-27  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [WebAssembly] Parse wasm modules in a streaming fashion
>Index: JSTests/microbenchmarks/dataview-cse.js
>===================================================================
>--- JSTests/microbenchmarks/dataview-cse.js	(nonexistent)
>+++ JSTests/microbenchmarks/dataview-cse.js	(working copy)
>@@ -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);
>+}
>Index: JSTests/stress/dataview-get-cse.js
>===================================================================
>--- JSTests/stress/dataview-get-cse.js	(nonexistent)
>+++ JSTests/stress/dataview-get-cse.js	(working copy)
>@@ -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: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 235475)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2018-08-29  Saam barati  <sbarati@apple.com>
>+
>+        CSE DataViewGet* DFG nodes
>+        https://bugs.webkit.org/show_bug.cgi?id=188768
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-29  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r235432 and r235436.
>Index: Source/JavaScriptCore/dfg/DFGClobberize.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGClobberize.h	(revision 235475)
>+++ Source/JavaScriptCore/dfg/DFGClobberize.h	(working copy)
>@@ -1763,6 +1763,9 @@ void clobberize(Graph& graph, Node* node
>     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: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGFixupPhase.cpp	(revision 235475)
>+++ Source/JavaScriptCore/dfg/DFGFixupPhase.cpp	(working copy)
>@@ -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: Source/JavaScriptCore/dfg/DFGHeapLocation.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGHeapLocation.cpp	(revision 235475)
>+++ Source/JavaScriptCore/dfg/DFGHeapLocation.cpp	(working copy)
>@@ -132,6 +132,10 @@ void printInternal(PrintStream& out, Loc
>         out.print("IndexedPropertyDoubleSaneChainLoc");
>         return;
> 
>+    case IndexedPropertyInt32Loc:
>+        out.print("IndexedPropertyInt32Loc");
>+        return;
>+
>     case IndexedPropertyInt52Loc:
>         out.print("IndexedPropertyInt52Loc");
>         return;
>Index: Source/JavaScriptCore/dfg/DFGHeapLocation.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGHeapLocation.h	(revision 235475)
>+++ Source/JavaScriptCore/dfg/DFGHeapLocation.h	(working copy)
>@@ -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 indexedPropertyLocFo
>         return IndexedPropertyDoubleLoc;
>     case NodeResultInt52:
>         return IndexedPropertyInt52Loc;
>+    case NodeResultInt32:
>+        return IndexedPropertyInt32Loc;
>     case NodeResultJS:
>         return IndexedPropertyJSLoc;
>     case NodeResultStorage:

Actions: View | Formatted Diff | Diff

Attachments on bug 188768: 348441