Attachment 357805 Details for Bug 192723 – Patch

[patch] Patch

bug-192723-20181220083733.patch (text/plain), 13.12 KB, created by Caio Lima on 2018-12-20 02:37:51 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2018-12-20 02:37:51 PST

Size: 13.12 KB

patch

obsolete

>Subversion Revision: 239437
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index d60fcf1ebe66043703430efc8392f45f2b124c51..47eb10853023d89dbf7c1089863ac1d3f370c794 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-12-20  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [BigInt] We should enable CSE into arithmetic operations that speculate BigIntUse
>+        https://bugs.webkit.org/show_bug.cgi?id=192723
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch is adjusting clobberize rules into ValueOp nodes to enable
>+        more optimizations when we speculate BigIntUse. In such case, DFG now
>+        is able to apply CSE, LICM and commutativity on nodes like
>+        ValueAdd(BigInt, BigInt), ValueSub(BigInt, BigInt), etc.
>+
>+        Here are the numbers we can observe with some microbenchmarks:
>+
>+                                  baseline                 changes
>+
>+        big-int-cse           108.2733+-0.8445    ^    80.9897+-4.9781   ^ definitely 1.3369x faster
>+        big-int-licm          75.6641+-0.3477     ^    57.8144+-1.6043   ^ definitely 1.3087x faster
>+        big-int-global-cse    145.3557+-1.0552    ^    86.5866+-0.3025   ^ definitely 1.6787x faster
>+
>+        * dfg/DFGAbstractInterpreterInlines.h:
>+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
>+        * dfg/DFGClobberize.h:
>+        (JSC::DFG::clobberize):
>+        * dfg/DFGStrengthReductionPhase.cpp:
>+        (JSC::DFG::StrengthReductionPhase::handleNode):
>+
> 2018-12-19  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239377.
>diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>index f288d6e6a7407ee11002d9d2eb85b6c2229add13..5197133ead4b2d28d1fb3ecfc547ff77f01ba905 100644
>--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>@@ -397,11 +397,12 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
>     case ValueBitXor:
>     case ValueBitAnd:
>     case ValueBitOr:
>-        clobberWorld();
>         if (node->binaryUseKind() == BigIntUse)
>             setTypeForNode(node, SpecBigInt);
>-        else
>+        else {
>+            clobberWorld();
>             setTypeForNode(node, SpecBoolInt32 | SpecBigInt);
>+        }
>         break;
>             
>     case ArithBitAnd:
>@@ -613,11 +614,12 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
>     case ValueSub:
>     case ValueAdd: {
>         DFG_ASSERT(m_graph, node, node->binaryUseKind() == UntypedUse || node->binaryUseKind() == BigIntUse);
>-        clobberWorld();
>         if (node->binaryUseKind() == BigIntUse)
>             setTypeForNode(node, SpecBigInt);
>-        else
>+        else {
>+            clobberWorld();
>             setTypeForNode(node, SpecString | SpecBytecodeNumber | SpecBigInt);
>+        }
>         break;
>     }
> 
>@@ -856,11 +858,12 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
>     }
>         
>     case ValueMul: {
>-        clobberWorld();
>         if (node->binaryUseKind() == BigIntUse)
>             setTypeForNode(node, SpecBigInt);
>-        else
>+        else {
>+            clobberWorld();
>             setTypeForNode(node, SpecBytecodeNumber | SpecBigInt);
>+        }
>         break;
>     }
> 
>@@ -915,11 +918,12 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
>     }
>         
>     case ValueDiv: {
>-        clobberWorld();
>         if (node->binaryUseKind() == BigIntUse)
>             setTypeForNode(node, SpecBigInt);
>-        else
>+        else {
>+            clobberWorld();
>             setTypeForNode(node, SpecBytecodeNumber | SpecBigInt);
>+        }
>         break;
>     }
> 
>diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
>index 250ec00443d12c063aa43eeebdc48c70294125a5..48c846bd3897d14de4e25b27d9570d82da871ae9 100644
>--- a/Source/JavaScriptCore/dfg/DFGClobberize.h
>+++ b/Source/JavaScriptCore/dfg/DFGClobberize.h
>@@ -644,14 +644,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
>     case InByVal:
>     case InById:
>     case HasOwnProperty:
>-    case ValueBitAnd:
>-    case ValueBitXor:
>-    case ValueBitOr:
>     case ValueNegate:
>-    case ValueAdd:
>-    case ValueSub:
>-    case ValueMul:
>-    case ValueDiv:
>     case SetFunctionName:
>     case GetDynamicVar:
>     case PutDynamicVar:
>@@ -673,6 +666,21 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
>         write(Heap);
>         return;
> 
>+    case ValueBitAnd:
>+    case ValueBitXor:
>+    case ValueBitOr:
>+    case ValueAdd:
>+    case ValueSub:
>+    case ValueMul:
>+    case ValueDiv:
>+        if (node->isBinaryUseKind(BigIntUse)) {
>+            def(PureValue(node));
>+            return;
>+        }
>+        read(World);
>+        write(Heap);
>+        return;
>+
>     case AtomicsAdd:
>     case AtomicsAnd:
>     case AtomicsCompareExchange:
>diff --git a/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp b/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp
>index 4f2f1d0b85fb3f70c225e47a59c4f706511e7d41..18bec693e2d0f70cd6dd5e90cb01a7c086d350b9 100644
>--- a/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp
>@@ -121,6 +121,15 @@ private:
>             }
>             break;
>             
>+        case ValueMul:
>+        case ValueBitOr:
>+        case ValueBitAnd:
>+        case ValueBitXor: {
>+            if (m_node->binaryUseKind() == BigIntUse)
>+                handleCommutativity();
>+            break;
>+        }
>+
>         case ArithMul: {
>             handleCommutativity();
>             Edge& child2 = m_node->child2();
>@@ -363,7 +372,12 @@ private:
>                 builder.append(rightString);
>                 convertToLazyJSValue(m_node, LazyJSValue::newString(m_graph, builder.toString()));
>                 m_changed = true;
>+                break;
>             }
>+
>+            if (m_node->binaryUseKind() == BigIntUse)
>+                handleCommutativity();
>+
>             break;
>         }
> 
>diff --git a/PerformanceTests/BigIntBench/big-int-cse.js b/PerformanceTests/BigIntBench/big-int-cse.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..d8b53dd7212746f590e15b66853270905ec52230
>--- /dev/null
>+++ b/PerformanceTests/BigIntBench/big-int-cse.js
>@@ -0,0 +1,103 @@
>+function assert(a, e) {
>+    if (a !== e)
>+        throw new Error("Expected " + e + " but got: " + a);
>+}
>+
>+function bigIntAdd(a, b) {
>+    let c = a + b;
>+    return b + a + c;
>+}
>+noInline(bigIntAdd);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntAdd(3n, 5n), 16n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntAdd(0xffffffffffffffffffn, 0xaaffffffffffffffffffn), 1624494070107157953511420n);
>+}
>+
>+function bigIntMul(a, b) {
>+    let c = a * b;
>+    return b * a + c;
>+}
>+noInline(bigIntMul);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntMul(3n, 5n), 30n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntMul(0xffffffffffffffffffn, 0xaaffffffffffffffffffn), 7626854857897473114403591155175632477091790850n);
>+}
>+
>+function bigIntDiv(a, b) {
>+    let c = a / b;
>+    return a / b + c;
>+}
>+noInline(bigIntDiv);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntDiv(15n, 5n), 6n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntDiv(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 342n);
>+}
>+
>+function bigIntSub(a, b) {
>+    let c = a - b;
>+    return a - b + c;
>+}
>+noInline(bigIntSub);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntSub(15n, 5n), 20n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntSub(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 1605604604175679372656640n);
>+}
>+
>+function bigIntBitOr(a, b) {
>+    let c = a | b;
>+    return (b | a) + c;
>+}
>+noInline(bigIntBitOr);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitOr(0b1101n, 0b0010n), 30n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitOr(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 1615049337141418663084030n);
>+}
>+
>+function bigIntBitAnd(a, b) {
>+    let c = a & b;
>+    return (b & a) + c;
>+}
>+noInline(bigIntBitAnd);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitAnd(0b1101n, 0b0010n), 0n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitAnd(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 9444732965739290427390n);
>+}
>+
>+function bigIntBitXor(a, b) {
>+    let c = a ^ b;
>+    return (b ^ a) + c;
>+}
>+noInline(bigIntBitXor);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitXor(0b1101n, 0b0010n), 30n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitXor(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 1605604604175679372656640n);
>+}
>+
>diff --git a/PerformanceTests/BigIntBench/big-int-global-cse.js b/PerformanceTests/BigIntBench/big-int-global-cse.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..d5c0246b263ca7770af1ee7081e6c5a3d4c17c3b
>--- /dev/null
>+++ b/PerformanceTests/BigIntBench/big-int-global-cse.js
>@@ -0,0 +1,124 @@
>+function assert(a, e) {
>+    if (a !== e)
>+        throw new Error("Expected " + e + " but got: " + a);
>+}
>+
>+function bigIntAdd(a, b) {
>+    let c = a + b;
>+    if (b) {
>+        assert(c, a + b);
>+    }
>+    return a + b + c;
>+}
>+noInline(bigIntAdd);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntAdd(3n, 5n), 16n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntAdd(0xffffffffffffffffffn, 0xaaffffffffffffffffffn), 1624494070107157953511420n);
>+}
>+
>+function bigIntMul(a, b) {
>+    let c = a * b;
>+    if (b) {
>+        assert(c, a * b);
>+    }
>+    return a * b + c;
>+}
>+noInline(bigIntMul);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntMul(3n, 5n), 30n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntMul(0xffffffffffffffffffn, 0xaaffffffffffffffffffn), 7626854857897473114403591155175632477091790850n);
>+}
>+
>+function bigIntDiv(a, b) {
>+    let c = a / b;
>+    if (b) {
>+        assert(c, a / b);
>+    }
>+    return a / b + c;
>+}
>+noInline(bigIntDiv);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntDiv(15n, 5n), 6n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntDiv(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 342n);
>+}
>+
>+function bigIntSub(a, b) {
>+    let c = a - b;
>+    if (b) {
>+        assert(c, a - b);
>+    }
>+    return a - b + c;
>+}
>+noInline(bigIntSub);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntSub(15n, 5n), 20n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntSub(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 1605604604175679372656640n);
>+}
>+
>+function bigIntBitOr(a, b) {
>+    let c = a | b;
>+    if (b) {
>+        assert(c, a | b);
>+    }
>+    return (a | b) + c;
>+}
>+noInline(bigIntBitOr);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitOr(0b1101n, 0b0010n), 30n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitOr(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 1615049337141418663084030n);
>+}
>+
>+function bigIntBitAnd(a, b) {
>+    let c = a & b;
>+    if (b) {
>+        assert(c, a & b);
>+    }
>+    return (a & b) + c;
>+}
>+noInline(bigIntBitAnd);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitAnd(0b1101n, 0b0010n), 0n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitAnd(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 9444732965739290427390n);
>+}
>+
>+function bigIntBitXor(a, b) {
>+    let c = a ^ b;
>+    if (b) {
>+        assert(c, a ^ b);
>+    }
>+    return (a ^ b) + c;
>+}
>+noInline(bigIntBitXor);
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitXor(0b1101n, 0b0010n), 30n);
>+}
>+
>+for (let i = 0; i < 100000; i++) {
>+    assert(bigIntBitXor(0xaaffffffffffffffffffn, 0xffffffffffffffffffn), 1605604604175679372656640n);
>+}
>+
>diff --git a/PerformanceTests/BigIntBench/big-int-licm.js b/PerformanceTests/BigIntBench/big-int-licm.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..6c64c9757983edc59e25e6bd912a049f07e6b2ce
>--- /dev/null
>+++ b/PerformanceTests/BigIntBench/big-int-licm.js
>@@ -0,0 +1,19 @@
>+function assert(a, e) {
>+    if (a !== e)
>+        throw new Error("Expected " + e + " but got: " + a);
>+}
>+
>+function iteration(a, b, r) {
>+    let acc = 0n;
>+    for (let i = 0n; i < r; i += 1n) {
>+        acc += a + b;
>+    }
>+
>+    return acc;
>+}
>+noInline(iteration);
>+
>+for (let i = 0; i < 10000; i++) {
>+    assert(iteration(1n, 2n, 100n), 300n)
>+}
>+
>diff --git a/PerformanceTests/ChangeLog b/PerformanceTests/ChangeLog
>index c115c5ba31af34ff96ba6cc966b319ddbd025c6e..47b4f63424d242c2cf8096b94fe2c1e5ec7ad38b 100644
>--- a/PerformanceTests/ChangeLog
>+++ b/PerformanceTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-12-20  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [BigInt] We should enable CSE into arithmetic operations that speculate BigIntUse
>+        https://bugs.webkit.org/show_bug.cgi?id=192723
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * BigIntBench/big-int-cse.js: Added.
>+        * BigIntBench/big-int-global-cse.js: Added.
>+        * BigIntBench/big-int-licm.js: Added.
>+
> 2018-12-19  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239377.

Actions: View | Formatted Diff | Diff

Attachments on bug 192723: 357436 | 357805