Attachment 359115 Details for Bug 192966 – Patch

[patch] Patch

bug-192966-20190114233419.patch (text/plain), 11.51 KB, created by Caio Lima on 2019-01-14 18:34:21 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2019-01-14 18:34:21 PST

Size: 11.51 KB

patch

obsolete

>Subversion Revision: 239960
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index cc5b39291b6d97b69d9188a35eb12dcc3fe635e6..0fec88afd51a08c76fdc4b978f59a4dc9478f2a4 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-01-14  Caio Lima  <ticaiolima@gmail.com>
>+
>+        DFGByteCodeParser rules for bitwise operations should consider type of their operands
>+        https://bugs.webkit.org/show_bug.cgi?id=192966
>+
>+        Reviewed by Yusuke Suzuki.
>+
>+        This patch is changing the logic how we lower bitwise operations, to
>+        consider only the type of input nodes and fix them during FixupPhase,
>+        if necessary. We are also changing the prediction propagation rules
>+        for ValueBitOp to use `getHeapPrediction()`. 
>+
>+        * dfg/DFGBackwardsPropagationPhase.cpp:
>+        (JSC::DFG::BackwardsPropagationPhase::isWithinPowerOfTwo):
>+        (JSC::DFG::BackwardsPropagationPhase::propagate):
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::parseBlock):
>+        * dfg/DFGFixupPhase.cpp:
>+        (JSC::DFG::FixupPhase::fixupNode):
>+        * dfg/DFGNode.h:
>+        (JSC::DFG::Node::hasInt32Result):
>+        (JSC::DFG::Node::hasNumberOrAnyIntResult):
>+        (JSC::DFG::Node::hasHeapPrediction):
>+        * dfg/DFGPredictionPropagationPhase.cpp:
>+
> 2019-01-14  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [JSC] Do not use asArrayModes() with Structures because it discards TypedArray information
>diff --git a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
>index 05c86ed1eb05f129ed4d00206d31e4e1f25d6159..f4ce71eba58adbd5b36109e6fb6b32962cdd72f9 100644
>--- a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
>@@ -110,6 +110,7 @@ private:
>             return isWithinPowerOfTwoForConstant<power>(node);
>         }
>             
>+        case ValueBitAnd:
>         case ArithBitAnd: {
>             if (power > 31)
>                 return true;
>@@ -120,6 +121,8 @@ private:
>             
>         case ArithBitOr:
>         case ArithBitXor:
>+        case ValueBitOr:
>+        case ValueBitXor:
>         case BitLShift: {
>             return power > 31;
>         }
>@@ -218,6 +221,9 @@ private:
>         case ArithBitAnd:
>         case ArithBitOr:
>         case ArithBitXor:
>+        case ValueBitAnd:
>+        case ValueBitOr:
>+        case ValueBitXor:
>         case BitRShift:
>         case BitLShift:
>         case BitURShift:
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index cfc583a0826835ec4048cdbd2ac7130907fbffa4..6b0d92a2f4354d4d823c10e45f6f478b98ed9cd0 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -4939,34 +4939,37 @@ void ByteCodeParser::parseBlock(unsigned limit)
> 
>         case op_bitand: {
>             auto bytecode = currentInstruction->as<OpBitand>();
>+            SpeculatedType prediction = getPrediction();
>             Node* op1 = get(bytecode.lhs);
>             Node* op2 = get(bytecode.rhs);
>-            if (isInt32Speculation(getPrediction()))
>+            if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
>                 set(bytecode.dst, addToGraph(ArithBitAnd, op1, op2));
>             else
>-                set(bytecode.dst, addToGraph(ValueBitAnd, op1, op2));
>+                set(bytecode.dst, addToGraph(ValueBitAnd, OpInfo(), OpInfo(prediction), op1, op2));
>             NEXT_OPCODE(op_bitand);
>         }
> 
>         case op_bitor: {
>             auto bytecode = currentInstruction->as<OpBitor>();
>+            SpeculatedType prediction = getPrediction();
>             Node* op1 = get(bytecode.lhs);
>             Node* op2 = get(bytecode.rhs);
>-            if (isInt32Speculation(getPrediction()))
>+            if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
>                 set(bytecode.dst, addToGraph(ArithBitOr, op1, op2));
>             else
>-                set(bytecode.dst, addToGraph(ValueBitOr, op1, op2));
>+                set(bytecode.dst, addToGraph(ValueBitOr, OpInfo(), OpInfo(prediction), op1, op2));
>             NEXT_OPCODE(op_bitor);
>         }
> 
>         case op_bitxor: {
>             auto bytecode = currentInstruction->as<OpBitxor>();
>+            SpeculatedType prediction = getPrediction();
>             Node* op1 = get(bytecode.lhs);
>             Node* op2 = get(bytecode.rhs);
>-            if (isInt32Speculation(getPrediction()))
>+            if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
>                 set(bytecode.dst, addToGraph(ArithBitXor, op1, op2));
>             else
>-                set(bytecode.dst, addToGraph(ValueBitXor, op1, op2));
>+                set(bytecode.dst, addToGraph(ValueBitXor, OpInfo(), OpInfo(prediction), op1, op2));
>             NEXT_OPCODE(op_bitor);
>         }
> 
>diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>index e6bac2df2ae6e36df3966be95353bbd1b1b69825..05112da4d6f5161403173eccd23dfc2f7091079b 100644
>--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>@@ -206,8 +206,32 @@ private:
>                 break;
>             }
> 
>-            fixEdge<UntypedUse>(node->child1());
>-            fixEdge<UntypedUse>(node->child2());
>+            if (Node::shouldSpeculateUntypedForBitOps(node->child1().node(), node->child2().node())) {
>+                fixEdge<UntypedUse>(node->child1());
>+                fixEdge<UntypedUse>(node->child2());
>+                break;
>+            }
>+
>+            // In such case, we need to fallback to ArithBitOp
>+            switch (op) {
>+            case ValueBitXor:
>+                node->setOp(ArithBitXor);
>+                break;
>+            case ValueBitOr:
>+                node->setOp(ArithBitOr);
>+                break;
>+            case ValueBitAnd:
>+                node->setOp(ArithBitAnd);
>+                break;
>+            default:
>+                DFG_CRASH(m_graph, node, "Unexpected node during ValueBit operation fixup");
>+                break;
>+            }
>+
>+            node->clearFlags(NodeMustGenerate);
>+            node->setResult(NodeResultInt32);
>+            fixIntConvertingEdge(node->child1());
>+            fixIntConvertingEdge(node->child2());
>             break;
>         }
> 
>@@ -225,26 +249,6 @@ private:
>         case ArithBitXor:
>         case ArithBitOr:
>         case ArithBitAnd: {
>-            if (Node::shouldSpeculateUntypedForBitOps(node->child1().node(), node->child2().node())) {
>-                fixEdge<UntypedUse>(node->child1());
>-                fixEdge<UntypedUse>(node->child2());
>-                switch (op) {
>-                case ArithBitXor:
>-                    node->setOpAndDefaultFlags(ValueBitXor);
>-                    break;
>-                case ArithBitOr:
>-                    node->setOpAndDefaultFlags(ValueBitOr);
>-                    break;
>-                case ArithBitAnd:
>-                    node->setOpAndDefaultFlags(ValueBitAnd);
>-                    break;
>-                default:
>-                    DFG_CRASH(m_graph, node, "Unexpected node during ArithBit operation fixup");
>-                    break;
>-                }
>-                break;
>-            }
>-
>             fixIntConvertingEdge(node->child1());
>             fixIntConvertingEdge(node->child2());
>             break;
>diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
>index b8014755c7bcb50914ed39d61a53d94aca86021e..0faaab723033bdc7bf4aa90db17757e12323d082 100644
>--- a/Source/JavaScriptCore/dfg/DFGNode.h
>+++ b/Source/JavaScriptCore/dfg/DFGNode.h
>@@ -1363,6 +1363,11 @@ public:
>         return !!result();
>     }
>     
>+    bool hasInt32Result()
>+    {
>+        return result() == NodeResultInt32;
>+    }
>+
>     bool hasInt52Result()
>     {
>         return result() == NodeResultInt52;
>@@ -1372,6 +1377,11 @@ public:
>     {
>         return result() == NodeResultNumber;
>     }
>+
>+    bool hasNumberOrAnyIntResult()
>+    {
>+        return hasNumberResult() || hasInt32Result() || hasInt52Result();
>+    }
>     
>     bool hasNumericResult()
>     {
>@@ -1685,6 +1695,9 @@ public:
>         case StringReplaceRegExp:
>         case ToNumber:
>         case ToObject:
>+        case ValueBitAnd:
>+        case ValueBitOr:
>+        case ValueBitXor:
>         case CallObjectConstructor:
>         case LoadKeyFromMapBucket:
>         case LoadValueFromMapBucket:
>diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>index a5869b3c24334bd7d135a28e721d2af43cd00acc..b0d49e08539baafadd215df51c784938cddee2f4 100644
>--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>@@ -277,14 +277,6 @@ private:
>             break;
>         }
> 
>-        case ValueBitXor:
>-        case ValueBitOr:
>-        case ValueBitAnd: {
>-            if (node->child1()->shouldSpeculateBigInt() && node->child2()->shouldSpeculateBigInt())
>-                changed |= mergePrediction(SpecBigInt);
>-            break;
>-        }
>-
>         case ValueNegate:
>         case ArithNegate: {
>             SpeculatedType prediction = node->child1()->prediction();
>@@ -807,6 +799,9 @@ private:
>         case LoadValueFromMapBucket:
>         case ToNumber:
>         case ToObject:
>+        case ValueBitAnd:
>+        case ValueBitXor:
>+        case ValueBitOr:
>         case CallObjectConstructor:
>         case GetArgument:
>         case CallDOMGetter:
>@@ -1118,9 +1113,6 @@ private:
>         case GetLocal:
>         case SetLocal:
>         case UInt32ToNumber:
>-        case ValueBitOr:
>-        case ValueBitAnd:
>-        case ValueBitXor:
>         case ValueNegate:
>         case ValueAdd:
>         case ValueSub:
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index b1f15c122d3f62d5e93e83a0263d50986a3f996c..602d8150fe71bfff4418ca6230a75b3f8fe7e3dd 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2019-01-14  Caio Lima  <ticaiolima@gmail.com>
>+
>+        DFGByteCodeParser rules for bitwise operations should consider type of their operands
>+        https://bugs.webkit.org/show_bug.cgi?id=192966
>+
>+        Reviewed by Yusuke Suzuki.
>+
>+        * stress/bit-op-with-object-returning-int32.js: Added.
>+
> 2019-01-14  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [JSC] Do not use asArrayModes() with Structures because it discards TypedArray information
>diff --git a/JSTests/stress/bit-op-with-object-returning-int32.js b/JSTests/stress/bit-op-with-object-returning-int32.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..8690c961efb8d9a28a31d5dda9d6fbc90641693f
>--- /dev/null
>+++ b/JSTests/stress/bit-op-with-object-returning-int32.js
>@@ -0,0 +1,37 @@
>+function assert(a, e) {
>+    if (a !== e)
>+        throw new Error("Expected: " + e + " but got: " + a);
>+}
>+
>+function bitAnd(a, b) {
>+    return a & b;
>+}
>+noInline(bitAnd);
>+
>+var o = { valueOf: () => 0b1101 };
>+
>+for (var i = 0; i < 10000; i++)
>+    assert(bitAnd(0b11, o), 0b1);
>+
>+assert(numberOfDFGCompiles(bitAnd) <= 1, true);
>+
>+function bitOr(a, b) {
>+    return a | b;
>+}
>+noInline(bitOr);
>+
>+for (var i = 0; i < 10000; i++)
>+    assert(bitOr(0b11, o), 0b1111);
>+
>+assert(numberOfDFGCompiles(bitOr) <= 1, true);
>+
>+function bitXor(a, b) {
>+    return a ^ b;
>+}
>+noInline(bitXor);
>+
>+for (var i = 0; i < 10000; i++)
>+    assert(bitXor(0b0011, o), 0b1110);
>+
>+assert(numberOfDFGCompiles(bitXor) <= 1, true);
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 192966: 357918 | 357919 | 358784 | 358786 | 359115