Attachment 357918 Details for Bug 192966 – Patch

[patch] Patch

bug-192966-20181221012006.patch (text/plain), 9.04 KB, created by Caio Lima on 2018-12-20 19:20:08 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2018-12-20 19:20:08 PST

Size: 9.04 KB

patch

obsolete

>Subversion Revision: 239478
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 5f116701d091550e6c534081df0c71a5382cc584..c64dde4afc1a6345ace56599d8f4e59b906c91ed 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-12-20  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, since they can produce better propagation when we
>+        consider their operands prediction. This change will remove the
>+        necessity of execution bitwise operations to properly emit AirthBitOp
>+        becaue we are not relying into `getPrediction()` call anymore.
>+
>+        * 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):
>+        * dfg/DFGPredictionPropagationPhase.cpp:
>+
> 2018-12-20  Chris Dumez  <cdumez@apple.com>
> 
>         Use Optional::hasValue() instead of Optional::has_value()
>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 d6e3843a8d8349490154e08777274f08655f650b..337396ae62a09ba2f05344b2ada07212a2acda30 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -4934,7 +4934,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             auto bytecode = currentInstruction->as<OpBitand>();
>             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));
>@@ -4945,7 +4945,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             auto bytecode = currentInstruction->as<OpBitor>();
>             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));
>@@ -4956,7 +4956,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             auto bytecode = currentInstruction->as<OpBitxor>();
>             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));
>diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>index 307632dad59da8e23250db1b4497d0341ba95292..ed243bc8bc62331defd54492fd655f28ca8282f9 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..3e9d16f2de880c6db7731a0b0e1405b72d6f43e8 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()
>     {
>diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>index b3c69bcf124727bfcc676b2afadab5a88c22f93a..8074bfe63ad000df5f11e2881a62b33c44343e54 100644
>--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>@@ -280,8 +280,17 @@ private:
>         case ValueBitXor:
>         case ValueBitOr:
>         case ValueBitAnd: {
>-            if (node->child1()->shouldSpeculateBigInt() && node->child2()->shouldSpeculateBigInt())
>-                changed |= mergePrediction(SpecBigInt);
>+            SpeculatedType left = node->child1()->prediction();
>+            SpeculatedType right = node->child2()->prediction();
>+
>+            if (left && right) {
>+                if (isBigIntSpeculation(left) && isBigIntSpeculation(right))
>+                    changed |= mergePrediction(SpecBigInt);
>+                else if (isFullNumberOrBooleanSpeculationExpectingDefined(left) && isFullNumberOrBooleanSpeculationExpectingDefined(right))
>+                    changed |= mergePrediction(SpecInt32Only);
>+                else
>+                    changed |= mergePrediction(SpecBigInt | SpecInt32Only);
>+            }
>             break;
>         }
>

Actions: View | Formatted Diff | Diff

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