Attachment 358784 Details for Bug 192966 – Patch

[patch] Patch

bug-192966-20190110104418.patch (text/plain), 9.82 KB, created by Caio Lima on 2019-01-10 05:44:20 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2019-01-10 05:44:20 PST

Size: 9.82 KB

patch

obsolete

>Subversion Revision: 239824
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index d01619c953099ed90c313fe3758a04aa4fc0d22a..3a887c7ab84d37301a2b155129c9fb7f382f106a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-01-10  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 NOBODY (OOPS!).
>+
>+        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-09  Mark Lam  <mark.lam@apple.com>
> 
>         Restore bytecode dumper's ability to dump jump target as offset#(->targetBytecodeIndex#).
>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 a6d3128f755ce8748a249bebb91f990000ecb0f5..60bc8fa1a0328f9cd26aebdba180e6e545d23c4b 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:

Actions: View | Formatted Diff | Diff

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