DFGByteCodeParser rules for bitwise operations should consider type of their operands
authorticaiolima@gmail.com <ticaiolima@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Jan 2019 10:14:31 +0000 (10:14 +0000)
committerticaiolima@gmail.com <ticaiolima@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Jan 2019 10:14:31 +0000 (10:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192966

Reviewed by Yusuke Suzuki.

JSTests:

* stress/bit-op-with-object-returning-int32.js: Added.

Source/JavaScriptCore:

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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239980 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/bit-op-with-object-returning-int32.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp

index a15e957..e2966c0 100644 (file)
@@ -1,3 +1,12 @@
+2019-01-15  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-15  Guillaume Emont  <guijemont@igalia.com>
 
         Skip a slow test and a flakey test on arm
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 (file)
index 0000000..8690c96
--- /dev/null
@@ -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);
+
index 2c8b556..9777693 100644 (file)
@@ -1,3 +1,28 @@
+2019-01-15  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-15  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Generate the DOMDebugger domain for Augmenting Agents (ObjC protocol)
index 05c86ed..f4ce71e 100644 (file)
@@ -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:
index cfc583a..6b0d92a 100644 (file)
@@ -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);
         }
 
index e6bac2d..05112da 100644 (file)
@@ -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;
index b801475..0faaab7 100644 (file)
@@ -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:
index a5869b3..b0d49e0 100644 (file)
@@ -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: