Attachment 362254 Details for Bug 186174 – Patch

[patch] Patch

bug-186174-20190217192502.patch (text/plain), 36.15 KB, created by Caio Lima on 2019-02-17 14:25:11 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2019-02-17 14:25:11 PST

Size: 36.15 KB

patch

obsolete

>Subversion Revision: 241651
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 165c1820f75845e5e77b2981132b8a8ff9decd53..84a2f7a4352d00243536d2d8990ac41747baef76 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,54 @@
>+2019-02-17  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [BigInt] Add ValueMod into DFG
>+        https://bugs.webkit.org/show_bug.cgi?id=186174
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch is introducing a new DFG node called ValueMod, that is
>+        responsible to handle BigInt and Untyped specialization of op_mod.
>+        With the introduction of BigInt, we think that cases where we can have
>+        ValueMod(Untyped, Untyped) can be common and we introduced
>+        support for such kind of node.
>+
>+        * dfg/DFGAbstractInterpreter.h:
>+        * dfg/DFGAbstractInterpreterInlines.h:
>+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::handleConstantDivOp):
>+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
>+        * dfg/DFGBackwardsPropagationPhase.cpp:
>+        (JSC::DFG::BackwardsPropagationPhase::propagate):
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::makeSafe):
>+        (JSC::DFG::ByteCodeParser::parseBlock):
>+        * dfg/DFGClobberize.h:
>+        (JSC::DFG::clobberize):
>+        * dfg/DFGDoesGC.cpp:
>+        (JSC::DFG::doesGC):
>+        * dfg/DFGFixupPhase.cpp:
>+        (JSC::DFG::FixupPhase::fixupArithDivInt32):
>+        (JSC::DFG::FixupPhase::fixupArithDiv):
>+        (JSC::DFG::FixupPhase::fixupNode):
>+        * dfg/DFGNodeType.h:
>+        * dfg/DFGOperations.cpp:
>+        (JSC::DFG::binaryOp):
>+        * dfg/DFGOperations.h:
>+        * dfg/DFGPredictionPropagationPhase.cpp:
>+        * dfg/DFGSafeToExecute.h:
>+        (JSC::DFG::safeToExecute):
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileValueMod):
>+        * dfg/DFGSpeculativeJIT.h:
>+        * dfg/DFGSpeculativeJIT32_64.cpp:
>+        (JSC::DFG::SpeculativeJIT::compile):
>+        * dfg/DFGSpeculativeJIT64.cpp:
>+        (JSC::DFG::SpeculativeJIT::compile):
>+        * dfg/DFGValidate.cpp:
>+        * ftl/FTLCapabilities.cpp:
>+        (JSC::FTL::canCompile):
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
>+        (JSC::FTL::DFG::LowerDFGToB3::compileValueMod):
>+
> 2019-02-17  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r241612.
>diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
>index fba129968994ebf0da26b6e72b8a13c8fbaa74e6..5d5988c29af2b87491c252e55c7eb241a297d4e6 100644
>--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
>+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
>@@ -253,6 +253,8 @@ private:
>     void verifyEdges(Node*);
>     void executeDoubleUnaryOpEffects(Node*, double(*equivalentFunction)(double));
>     
>+    bool handleConstantDivOp(Node*);
>+
>     CodeBlock* m_codeBlock;
>     Graph& m_graph;
>     VM& m_vm;
>diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>index 67e5a87cb3880dfe5bfca58615ef3bbab3d08ad6..915b6470140b4558ea588199456e76f8f895b070 100644
>--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>@@ -232,6 +232,52 @@ inline ToThisResult isToThisAnIdentity(VM& vm, bool isStrictMode, AbstractValue&
>     return ToThisResult::Dynamic;
> }
> 
>+template<typename AbstractStateType>
>+bool AbstractInterpreter<AbstractStateType>::handleConstantDivOp(Node* node)
>+{
>+    JSValue left = forNode(node->child1()).value();
>+    JSValue right = forNode(node->child2()).value();
>+
>+    if (left && right) {
>+        NodeType op = node->op();
>+        if (left.isInt32() && right.isInt32()) {
>+            double doubleResult;
>+            if (op == ValueDiv || op == ArithDiv)
>+                doubleResult = left.asNumber() / right.asNumber();
>+            else
>+                doubleResult = fmod(left.asNumber(), right.asNumber());
>+
>+            if (node->hasArithMode()) {
>+                if (!shouldCheckOverflow(node->arithMode()))
>+                    doubleResult = toInt32(doubleResult);
>+                else if (!shouldCheckNegativeZero(node->arithMode()))
>+                    doubleResult += 0; // Sanitizes zero.
>+            }
>+
>+            JSValue valueResult = jsNumber(doubleResult);
>+            if (valueResult.isInt32()) {
>+                if (op == ValueDiv || op == ValueMod)
>+                    didFoldClobberWorld();
>+                setConstant(node, valueResult);
>+                return true;
>+            }
>+        } else if (left.isNumber() && right.isNumber()) {
>+            if (op == ValueDiv || op == ValueMod)
>+                didFoldClobberWorld();
>+
>+            if (op == ValueDiv || op == ArithDiv)
>+                setConstant(node, jsDoubleNumber(left.asNumber() / right.asNumber()));
>+            else
>+                setConstant(node, jsDoubleNumber(fmod(left.asNumber(), right.asNumber())));
>+
>+            return true;
>+        }
>+    }
>+
>+    return false;
>+}
>+
>+
> template<typename AbstractStateType>
> bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimit, Node* node)
> {
>@@ -917,7 +963,11 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
>         break;
>     }
>         
>+    case ValueMod:
>     case ValueDiv: {
>+        if (handleConstantDivOp(node))
>+            break;
>+
>         if (node->binaryUseKind() == BigIntUse)
>             setTypeForNode(node, SpecBigInt);
>         else {
>@@ -927,68 +977,26 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
>         break;
>     }
> 
>+    case ArithMod:
>     case ArithDiv: {
>-        JSValue left = forNode(node->child1()).value();
>-        JSValue right = forNode(node->child2()).value();
>-        switch (node->binaryUseKind()) {
>-        case Int32Use:
>-            if (left && right && left.isInt32() && right.isInt32()) {
>-                double doubleResult = left.asNumber() / right.asNumber();
>-                if (!shouldCheckOverflow(node->arithMode()))
>-                    doubleResult = toInt32(doubleResult);
>-                else if (!shouldCheckNegativeZero(node->arithMode()))
>-                    doubleResult += 0; // Sanitizes zero.
>-                JSValue valueResult = jsNumber(doubleResult);
>-                if (valueResult.isInt32()) {
>-                    setConstant(node, valueResult);
>-                    break;
>-                }
>-            }
>-            setNonCellTypeForNode(node, SpecInt32Only);
>+        if (handleConstantDivOp(node))
>             break;
>-        case DoubleRepUse:
>-            if (left && right && left.isNumber() && right.isNumber()) {
>-                setConstant(node, jsDoubleNumber(left.asNumber() / right.asNumber()));
>-                break;
>-            }
>-            setNonCellTypeForNode(node, 
>-                typeOfDoubleQuotient(
>-                    forNode(node->child1()).m_type, forNode(node->child2()).m_type));
>-            break;
>-        default:
>-            RELEASE_ASSERT_NOT_REACHED();
>-            break;
>-        }
>-        break;
>-    }
> 
>-    case ArithMod: {
>-        JSValue left = forNode(node->child1()).value();
>-        JSValue right = forNode(node->child2()).value();
>         switch (node->binaryUseKind()) {
>         case Int32Use:
>-            if (left && right && left.isInt32() && right.isInt32()) {
>-                double doubleResult = fmod(left.asNumber(), right.asNumber());
>-                if (!shouldCheckOverflow(node->arithMode()))
>-                    doubleResult = toInt32(doubleResult);
>-                else if (!shouldCheckNegativeZero(node->arithMode()))
>-                    doubleResult += 0; // Sanitizes zero.
>-                JSValue valueResult = jsNumber(doubleResult);
>-                if (valueResult.isInt32()) {
>-                    setConstant(node, valueResult);
>-                    break;
>-                }
>-            }
>             setNonCellTypeForNode(node, SpecInt32Only);
>             break;
>         case DoubleRepUse:
>-            if (left && right && left.isNumber() && right.isNumber()) {
>-                setConstant(node, jsDoubleNumber(fmod(left.asNumber(), right.asNumber())));
>-                break;
>+            if (node->op() == ArithDiv) {
>+                setNonCellTypeForNode(node, 
>+                    typeOfDoubleQuotient(
>+                        forNode(node->child1()).m_type, forNode(node->child2()).m_type));
>+            } else {
>+                setNonCellTypeForNode(node, 
>+                    typeOfDoubleBinaryOp(
>+                        forNode(node->child1()).m_type, forNode(node->child2()).m_type));
>             }
>-            setNonCellTypeForNode(node, 
>-                typeOfDoubleBinaryOp(
>-                    forNode(node->child1()).m_type, forNode(node->child2()).m_type));
>+            
>             break;
>         default:
>             RELEASE_ASSERT_NOT_REACHED();
>diff --git a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
>index f4ce71eba58adbd5b36109e6fb6b32962cdd72f9..504d0442a9fa135609e61a8880728c711c721d45 100644
>--- a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
>@@ -362,6 +362,7 @@ private:
>             break;
>         }
>             
>+        case ValueMod:
>         case ArithMod: {
>             flags |= NodeBytecodeUsesAsNumber;
>             flags &= ~NodeBytecodeUsesAsOther;
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index 77840e89c54e81e287fb3f653ce758ea8d20a080..6858012d15d99270a7b4c2fde5c7a46fbae626d5 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -930,7 +930,7 @@ private:
>         if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, NegativeZero))
>             node->mergeFlags(NodeMayNegZeroInDFG);
>         
>-        if (!isX86() && node->op() == ArithMod)
>+        if (!isX86() && (node->op() == ArithMod || node->op() == ValueMod))
>             return node;
> 
>         {
>@@ -991,6 +991,7 @@ private:
>             case ArithAdd:
>             case ArithSub:
>             case ValueAdd:
>+            case ValueMod:
>             case ArithMod: // for ArithMod "MayOverflow" means we tried to divide by zero, or we saw double.
>                 node->mergeFlags(NodeMayOverflowInt32InBaseline);
>                 break;
>@@ -5055,7 +5056,10 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             auto bytecode = currentInstruction->as<OpMod>();
>             Node* op1 = get(bytecode.m_lhs);
>             Node* op2 = get(bytecode.m_rhs);
>-            set(bytecode.m_dst, makeSafe(addToGraph(ArithMod, op1, op2)));
>+            if (op1->hasNumberResult() && op2->hasNumberResult())
>+                set(bytecode.m_dst, makeSafe(addToGraph(ArithMod, op1, op2)));
>+            else
>+                set(bytecode.m_dst, makeSafe(addToGraph(ValueMod, op1, op2)));
>             NEXT_OPCODE(op_mod);
>         }
> 
>diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
>index add1499e8a69ae192a13d1761ffbd27b72bd8f7a..db2d72ed371f5e177a1059a9a5f392af7bcf61dc 100644
>--- a/Source/JavaScriptCore/dfg/DFGClobberize.h
>+++ b/Source/JavaScriptCore/dfg/DFGClobberize.h
>@@ -670,6 +670,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
>     case ValueSub:
>     case ValueMul:
>     case ValueDiv:
>+    case ValueMod:
>         if (node->isBinaryUseKind(BigIntUse)) {
>             def(PureValue(node));
>             return;
>diff --git a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>index 1598f7e85809bd769c9891958aeaff69e66a0386..d76143dc1c6e4a9664823af7d146d89e45000732 100644
>--- a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>@@ -384,6 +384,7 @@ bool doesGC(Graph& graph, Node* node)
>     case ValueSub:
>     case ValueMul:
>     case ValueDiv:
>+    case ValueMod:
>     case ValueNegate:
> #else
>     // See comment at the top for why be default for all nodes should be to
>diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>index c68d6e2dd8d4e7ea338a6a7b54be5150588f3df2..8e981c38d83880a37fec01f84fe523653f1318fd 100644
>--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>@@ -75,38 +75,45 @@ public:
>     }
> 
> private:
>-    void fixupArithDiv(Node* node, Edge& leftChild, Edge& rightChild)
>+
>+    void fixupArithDivInt32(Node* node, Edge& leftChild, Edge& rightChild)
>     {
>-        if (m_graph.binaryArithShouldSpeculateInt32(node, FixupPass)) {
>-            if (optimizeForX86() || optimizeForARM64() || optimizeForARMv7IDIVSupported()) {
>-                fixIntOrBooleanEdge(leftChild);
>-                fixIntOrBooleanEdge(rightChild);
>-                if (bytecodeCanTruncateInteger(node->arithNodeFlags()))
>-                    node->setArithMode(Arith::Unchecked);
>-                else if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags()))
>-                    node->setArithMode(Arith::CheckOverflow);
>-                else
>-                    node->setArithMode(Arith::CheckOverflowAndNegativeZero);
>-                return;
>-            }
>-            
>-            // This will cause conversion nodes to be inserted later.
>-            fixDoubleOrBooleanEdge(leftChild);
>-            fixDoubleOrBooleanEdge(rightChild);
>-            
>-            // We don't need to do ref'ing on the children because we're stealing them from
>-            // the original division.
>-            Node* newDivision = m_insertionSet.insertNode(m_indexInBlock, SpecBytecodeDouble, *node);
>-            newDivision->setResult(NodeResultDouble);
>-            
>-            node->setOp(DoubleAsInt32);
>-            node->children.initialize(Edge(newDivision, DoubleRepUse), Edge(), Edge());
>-            if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags()))
>+        if (optimizeForX86() || optimizeForARM64() || optimizeForARMv7IDIVSupported()) {
>+            fixIntOrBooleanEdge(leftChild);
>+            fixIntOrBooleanEdge(rightChild);
>+            if (bytecodeCanTruncateInteger(node->arithNodeFlags()))
>+                node->setArithMode(Arith::Unchecked);
>+            else if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags()))
>                 node->setArithMode(Arith::CheckOverflow);
>             else
>                 node->setArithMode(Arith::CheckOverflowAndNegativeZero);
>             return;
>         }
>+
>+        // This will cause conversion nodes to be inserted later.
>+        fixDoubleOrBooleanEdge(leftChild);
>+        fixDoubleOrBooleanEdge(rightChild);
>+
>+        // We don't need to do ref'ing on the children because we're stealing them from
>+        // the original division.
>+        Node* newDivision = m_insertionSet.insertNode(m_indexInBlock, SpecBytecodeDouble, *node);
>+        newDivision->setResult(NodeResultDouble);
>+
>+        node->setOp(DoubleAsInt32);
>+        node->children.initialize(Edge(newDivision, DoubleRepUse), Edge(), Edge());
>+        if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags()))
>+            node->setArithMode(Arith::CheckOverflow);
>+        else
>+            node->setArithMode(Arith::CheckOverflowAndNegativeZero);
>+
>+    }
>+
>+    void fixupArithDiv(Node* node, Edge& leftChild, Edge& rightChild)
>+    {
>+        if (m_graph.binaryArithShouldSpeculateInt32(node, FixupPass)) {
>+            fixupArithDivInt32(node, leftChild, rightChild);
>+            return;
>+        }
>         
>         fixDoubleOrBooleanEdge(leftChild);
>         fixDoubleOrBooleanEdge(rightChild);
>@@ -495,6 +502,7 @@ private:
>             break;
>         }
> 
>+        case ValueMod: 
>         case ValueDiv: {
>             Edge& leftChild = node->child1();
>             Edge& rightChild = node->child2();
>@@ -510,7 +518,12 @@ private:
>                 fixEdge<UntypedUse>(rightChild);
>                 break;
>             }
>-            node->setOp(ArithDiv);
>+
>+            if (op == ValueDiv)
>+                node->setOp(ArithDiv);
>+            else
>+                node->setOp(ArithMod);
>+
>             node->setResult(NodeResultNumber);
>             fixupArithDiv(node, leftChild, rightChild);
>             break;
>diff --git a/Source/JavaScriptCore/dfg/DFGNodeType.h b/Source/JavaScriptCore/dfg/DFGNodeType.h
>index ef63cdb446c74af9c66631ef3b17b28469590d59..a45ee260f5ff195ddfba05f70ab75cd2fa77deeb 100644
>--- a/Source/JavaScriptCore/dfg/DFGNodeType.h
>+++ b/Source/JavaScriptCore/dfg/DFGNodeType.h
>@@ -176,6 +176,7 @@ namespace JSC { namespace DFG {
>     macro(ValueSub, NodeResultJS | NodeMustGenerate) \
>     macro(ValueMul, NodeResultJS | NodeMustGenerate) \
>     macro(ValueDiv, NodeResultJS | NodeMustGenerate) \
>+    macro(ValueMod, NodeResultJS | NodeMustGenerate) \
>     \
>     /* Add of values that always convers its inputs to strings. May have two or three kids. */\
>     macro(StrCat, NodeResultJS | NodeMustGenerate) \
>diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>index 6a578266eb4a128327e53cdaacb7d861577c49ef..6f10d8bbd1c7398169ea354f3aa9cc6398cb377c 100644
>--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>@@ -212,6 +212,33 @@ static ALWAYS_INLINE void putWithThis(ExecState* exec, EncodedJSValue encodedBas
>     baseValue.putInline(exec, ident, putValue, slot);
> }
> 
>+template<typename BigIntOperation, typename NumberOperation>
>+static ALWAYS_INLINE EncodedJSValue binaryOp(ExecState* exec, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2, BigIntOperation&& bigIntOp, NumberOperation&& numberOp, const char* errorMessage)
>+{
>+    VM* vm = &exec->vm();
>+    NativeCallFrameTracer tracer(vm, exec);
>+    auto scope = DECLARE_THROW_SCOPE(*vm);
>+
>+    JSValue op1 = JSValue::decode(encodedOp1);
>+    JSValue op2 = JSValue::decode(encodedOp2);
>+
>+    auto leftNumeric = op1.toNumeric(exec);
>+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+    auto rightNumeric = op2.toNumeric(exec);
>+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+
>+    if (WTF::holds_alternative<JSBigInt*>(leftNumeric) || WTF::holds_alternative<JSBigInt*>(rightNumeric)) {
>+        if (WTF::holds_alternative<JSBigInt*>(leftNumeric) && WTF::holds_alternative<JSBigInt*>(rightNumeric))
>+            RELEASE_AND_RETURN(scope, JSValue::encode(bigIntOp(exec, WTF::get<JSBigInt*>(leftNumeric), WTF::get<JSBigInt*>(rightNumeric))));
>+
>+        return throwVMTypeError(exec, scope, errorMessage);
>+    }
>+
>+    scope.release();
>+
>+    return JSValue::encode(jsNumber(numberOp(WTF::get<double>(leftNumeric), WTF::get<double>(rightNumeric))));
>+}
>+
> static ALWAYS_INLINE EncodedJSValue parseIntResult(double input)
> {
>     int asInt = static_cast<int>(input);
>@@ -352,6 +379,19 @@ JSCell* JIT_OPERATION operationToObject(ExecState* exec, JSGlobalObject* globalO
>     RELEASE_AND_RETURN(scope, value.toObject(exec, globalObject));
> }
> 
>+EncodedJSValue JIT_OPERATION operationValueMod(ExecState* exec, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2)
>+{
>+    auto bigIntOp = [] (ExecState* exec, JSBigInt* left, JSBigInt* right) -> JSBigInt* {
>+        return JSBigInt::remainder(exec, left, right);
>+    };
>+
>+    auto numberOp = [] (double left, double right) -> double {
>+        return jsMod(left, right);
>+    };
>+
>+    return binaryOp(exec, encodedOp1, encodedOp2, bigIntOp, numberOp, "Invalid mix of BigInt and other type in remainder operation.");
>+}
>+
> EncodedJSValue JIT_OPERATION operationValueBitNot(ExecState* exec, EncodedJSValue encodedOp1)
> {
>     VM* vm = &exec->vm();
>@@ -508,33 +548,15 @@ EncodedJSValue JIT_OPERATION operationValueAddNotNumber(ExecState* exec, Encoded
> 
> EncodedJSValue JIT_OPERATION operationValueDiv(ExecState* exec, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2)
> {
>-    VM* vm = &exec->vm();
>-    NativeCallFrameTracer tracer(vm, exec);
>-    auto scope = DECLARE_THROW_SCOPE(*vm);
>-
>-    JSValue op1 = JSValue::decode(encodedOp1);
>-    JSValue op2 = JSValue::decode(encodedOp2);
>-
>-    auto leftNumeric = op1.toNumeric(exec);
>-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
>-    auto rightNumeric = op2.toNumeric(exec);
>-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+    auto bigIntOp = [] (ExecState* exec, JSBigInt* left, JSBigInt* right) -> JSBigInt* {
>+        return JSBigInt::divide(exec, left, right);
>+    };
> 
>-    if (WTF::holds_alternative<JSBigInt*>(leftNumeric) || WTF::holds_alternative<JSBigInt*>(rightNumeric)) {
>-        if (WTF::holds_alternative<JSBigInt*>(leftNumeric) && WTF::holds_alternative<JSBigInt*>(rightNumeric)) {
>-            JSBigInt* result = JSBigInt::divide(exec, WTF::get<JSBigInt*>(leftNumeric), WTF::get<JSBigInt*>(rightNumeric));
>-            RETURN_IF_EXCEPTION(scope, encodedJSValue());
>-            return JSValue::encode(result);
>-        }
>+    auto numberOp = [] (double left, double right) -> double {
>+        return left / right;
>+    };
> 
>-        return throwVMTypeError(exec, scope, "Invalid mix of BigInt and other type in division operation.");
>-    }
>-
>-    scope.release();
>-
>-    double a = WTF::get<double>(leftNumeric);
>-    double b = WTF::get<double>(rightNumeric);
>-    return JSValue::encode(jsNumber(a / b));
>+    return binaryOp(exec, encodedOp1, encodedOp2, bigIntOp, numberOp, "Invalid mix of BigInt and other type in division operation.");
> }
> 
> double JIT_OPERATION operationArithAbs(ExecState* exec, EncodedJSValue encodedOp1)
>@@ -1344,6 +1366,17 @@ JSCell* JIT_OPERATION operationMulBigInt(ExecState* exec, JSCell* op1, JSCell* o
>     return JSBigInt::multiply(exec, leftOperand, rightOperand);
> }
>     
>+JSCell* JIT_OPERATION operationModBigInt(ExecState* exec, JSCell* op1, JSCell* op2)
>+{
>+    VM* vm = &exec->vm();
>+    NativeCallFrameTracer tracer(vm, exec);
>+    
>+    JSBigInt* leftOperand = jsCast<JSBigInt*>(op1);
>+    JSBigInt* rightOperand = jsCast<JSBigInt*>(op2);
>+    
>+    return JSBigInt::remainder(exec, leftOperand, rightOperand);
>+}
>+
> JSCell* JIT_OPERATION operationDivBigInt(ExecState* exec, JSCell* op1, JSCell* op2)
> {
>     VM* vm = &exec->vm();
>diff --git a/Source/JavaScriptCore/dfg/DFGOperations.h b/Source/JavaScriptCore/dfg/DFGOperations.h
>index 38c116a54f91a615b81c15112148f25f94adb1ef..9d7cf2ad3c9a35f29238dbbff5b8b5bb615eebfa 100644
>--- a/Source/JavaScriptCore/dfg/DFGOperations.h
>+++ b/Source/JavaScriptCore/dfg/DFGOperations.h
>@@ -50,6 +50,7 @@ JSCell* JIT_OPERATION operationObjectCreateObject(ExecState*, JSObject*) WTF_INT
> JSCell* JIT_OPERATION operationCreateThis(ExecState*, JSObject* constructor, uint32_t inlineCapacity) WTF_INTERNAL;
> EncodedJSValue JIT_OPERATION operationToThis(ExecState*, EncodedJSValue encodedOp1) WTF_INTERNAL;
> EncodedJSValue JIT_OPERATION operationToThisStrict(ExecState*, EncodedJSValue encodedOp1) WTF_INTERNAL;
>+EncodedJSValue JIT_OPERATION operationValueMod(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
> EncodedJSValue JIT_OPERATION operationValueBitNot(ExecState*, EncodedJSValue encodedOp1) WTF_INTERNAL;
> EncodedJSValue JIT_OPERATION operationValueBitAnd(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
> EncodedJSValue JIT_OPERATION operationValueBitOr(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
>@@ -168,6 +169,7 @@ size_t JIT_OPERATION operationRegExpTestGeneric(ExecState*, JSGlobalObject*, Enc
> size_t JIT_OPERATION operationCompareStrictEqCell(ExecState*, JSCell* op1, JSCell* op2) WTF_INTERNAL;
> JSCell* JIT_OPERATION operationSubBigInt(ExecState*, JSCell* op1, JSCell* op2) WTF_INTERNAL;
> JSCell* JIT_OPERATION operationMulBigInt(ExecState*, JSCell* op1, JSCell* op2) WTF_INTERNAL;
>+JSCell* JIT_OPERATION operationModBigInt(ExecState*, JSCell* op1, JSCell* op2) WTF_INTERNAL;
> JSCell* JIT_OPERATION operationDivBigInt(ExecState*, JSCell* op1, JSCell* op2) WTF_INTERNAL;
> JSCell* JIT_OPERATION operationBitAndBigInt(ExecState*, JSCell* op1, JSCell* op2) WTF_INTERNAL;
> JSCell* JIT_OPERATION operationBitOrBigInt(ExecState*, JSCell* op1, JSCell* op2) WTF_INTERNAL;
>diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>index c2afe5b378add7ae4d1e9bd4fd8320ae27687065..cbd9796b63967fae6cc68ce5017913f54e258c5b 100644
>--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>@@ -348,6 +348,7 @@ private:
>         }
> 
>         case ValueDiv:
>+        case ValueMod:
>         case ArithDiv:
>         case ArithMod: {
>             SpeculatedType left = node->child1()->prediction();
>@@ -360,11 +361,11 @@ private:
>                         changed |= mergePrediction(SpecInt32Only);
>                     else
>                         changed |= mergePrediction(SpecBytecodeDouble);
>-                } else if (op == ValueDiv && isBigIntSpeculation(left) && isBigIntSpeculation(right))
>+                } else if ((op == ValueDiv || op == ValueMod) && isBigIntSpeculation(left) && isBigIntSpeculation(right))
>                     changed |= mergePrediction(SpecBigInt);
>                 else {
>                     changed |= mergePrediction(SpecInt32Only | SpecBytecodeDouble);
>-                    if (op == ValueDiv && (node->mayHaveBigIntResult()
>+                    if ((op == ValueDiv || op == ValueMod) && (node->mayHaveBigIntResult()
>                         || (left & SpecBigInt)
>                         || (right & SpecBigInt)))
>                         changed |= mergePrediction(SpecBigInt);
>@@ -601,6 +602,7 @@ private:
>         case ArithMax:
>         case ArithMod:
>         case ValueDiv:
>+        case ValueMod:
>         case ArithDiv: {
>             SpeculatedType left = node->child1()->prediction();
>             SpeculatedType right = node->child2()->prediction();
>@@ -1117,6 +1119,7 @@ private:
>         case ValueSub:
>         case ValueMul:
>         case ValueDiv:
>+        case ValueMod:
>         case ArithAdd:
>         case ArithSub:
>         case ArithNegate:
>diff --git a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
>index 883f79988bd6a4a4c2735173bc5c598c5a77666d..92a1334d62b79006c48aed062cb5169a187b18be 100644
>--- a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
>+++ b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
>@@ -236,6 +236,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node, bool igno
>     case ValueSub:
>     case ValueMul:
>     case ValueDiv:
>+    case ValueMod:
>     case TryGetById:
>     case DeleteById:
>     case DeleteByVal:
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index 2911288ede8d5699066d0d11ff2b7fd4b29de121..cd1987d9dc98492ed5f26aa596b8608009741ae7 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -5263,6 +5263,44 @@ void SpeculativeJIT::compileArithFRound(Node* node)
>     doubleResult(result.fpr(), node);
> }
> 
>+void SpeculativeJIT::compileValueMod(Node* node)
>+{
>+    Edge& leftChild = node->child1();
>+    Edge& rightChild = node->child2();
>+
>+    if (leftChild.useKind() == BigIntUse && rightChild.useKind() == BigIntUse) {
>+        SpeculateCellOperand left(this, leftChild);
>+        SpeculateCellOperand right(this, rightChild);
>+        GPRReg leftGPR = left.gpr();
>+        GPRReg rightGPR = right.gpr();
>+
>+        speculateBigInt(leftChild, leftGPR);
>+        speculateBigInt(rightChild, rightGPR);
>+
>+        flushRegisters();
>+        GPRFlushedCallResult result(this);
>+        GPRReg resultGPR = result.gpr();
>+
>+        callOperation(operationModBigInt, resultGPR, leftGPR, rightGPR);
>+
>+        m_jit.exceptionCheck();
>+        cellResult(resultGPR, node);
>+        return;
>+    }
>+
>+    DFG_ASSERT(m_jit.graph(), node, node->binaryUseKind() == UntypedUse, node->binaryUseKind());
>+    JSValueOperand op1(this, leftChild);
>+    JSValueOperand op2(this, rightChild);
>+    JSValueRegs op1Regs = op1.jsValueRegs();
>+    JSValueRegs op2Regs = op2.jsValueRegs();
>+    flushRegisters();
>+    JSValueRegsFlushedCallResult result(this);
>+    JSValueRegs resultRegs = result.regs();
>+    callOperation(operationValueMod, resultRegs, op1Regs, op2Regs);
>+    m_jit.exceptionCheck();
>+    jsValueResult(resultRegs, node);
>+}
>+
> void SpeculativeJIT::compileArithMod(Node* node)
> {
>     switch (node->binaryUseKind()) {
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>index 8ed85f6311f14a940530e2f146288ab612b10b3d..5d7735aa6b8a63e10aab1b24ce5ab867aa48e666 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>@@ -1360,6 +1360,7 @@ public:
>     void compileValueDiv(Node*);
>     void compileArithDiv(Node*);
>     void compileArithFRound(Node*);
>+    void compileValueMod(Node*);
>     void compileArithMod(Node*);
>     void compileArithPow(Node*);
>     void compileArithRounding(Node*);
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>index 4dfe9c92321d753d178148f4ed35f8973f3e6f52..e4eb0b0ecdd6687b55ea773dcdf401f1e1ededa7 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>@@ -2081,6 +2081,11 @@ void SpeculativeJIT::compile(Node* node)
>         break;
>     }
> 
>+    case ValueMod: {
>+        compileValueMod(node);
>+        break;
>+    }
>+
>     case ArithMod: {
>         compileArithMod(node);
>         break;
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>index 63052becc0508d6d2927cb0060b4be87626976a1..06ec4227715fda2e4ad7b0793084a706f230ddf4 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>@@ -2223,6 +2223,11 @@ void SpeculativeJIT::compile(Node* node)
>         break;
>     }
> 
>+    case ValueMod: {
>+        compileValueMod(node);
>+        break;
>+    }
>+
>     case ArithMod: {
>         compileArithMod(node);
>         break;
>diff --git a/Source/JavaScriptCore/dfg/DFGValidate.cpp b/Source/JavaScriptCore/dfg/DFGValidate.cpp
>index abd1ab2f512c35677cad4f4c95fdd5fb94ec15ce..9118058240a72f3e81ff427be21cc02a945e7f58 100644
>--- a/Source/JavaScriptCore/dfg/DFGValidate.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGValidate.cpp
>@@ -257,6 +257,7 @@ public:
>                 case ValueSub:
>                 case ValueMul:
>                 case ValueDiv:
>+                case ValueMod:
>                 case ArithAdd:
>                 case ArithSub:
>                 case ArithMul:
>diff --git a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
>index 535721296925a9009042f111048a9986f8b7d220..515abd8a0c25867ab98afc6d6161ed4e13fff290 100644
>--- a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
>@@ -96,6 +96,7 @@ inline CapabilityLevel canCompile(Node* node)
>     case ValueSub:
>     case ValueMul:
>     case ValueDiv:
>+    case ValueMod:
>     case StrCat:
>     case ArithAdd:
>     case ArithClz32:
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index a17f95421f7c30b5704e77d2ffd8a6a90317e1b1..c2222d9f8d862365174baf42d238c58a710487d1 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -619,6 +619,9 @@ private:
>         case ArithDiv:
>             compileArithDiv();
>             break;
>+        case ValueMod:
>+            compileValueMod();
>+            break;
>         case ArithMod:
>             compileArithMod();
>             break;
>@@ -2370,6 +2373,24 @@ private:
>         }
>     }
>     
>+    void compileValueMod()
>+    {
>+        if (m_node->binaryUseKind() == BigIntUse) {
>+            LValue left = lowBigInt(m_node->child1());
>+            LValue right = lowBigInt(m_node->child2());
>+
>+            LValue result = vmCall(pointerType(), m_out.operation(operationModBigInt), m_callFrame, left, right);
>+            setJSValue(result);
>+            return;
>+        }
>+
>+        DFG_ASSERT(m_graph, m_node, m_node->binaryUseKind() == UntypedUse, m_node->binaryUseKind());
>+        LValue left = lowJSValue(m_node->child1());
>+        LValue right = lowJSValue(m_node->child2());
>+        LValue result = vmCall(Int64, m_out.operation(operationValueMod), m_callFrame, left, right);
>+        setJSValue(result);
>+    }
>+
>     void compileArithMod()
>     {
>         switch (m_node->binaryUseKind()) {
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 1c9bc23e4b40e5abcc4f8259e6abe2734f8fe86d..4af83e30f09697131ab5aac6a8565170ba1b1439 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-02-17  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [BigInt] Add ValueMod into DFG
>+        https://bugs.webkit.org/show_bug.cgi?id=186174
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * microbenchmarks/mod-untyped.js: Added.
>+        * stress/big-int-mod-osr.js: Added.
>+
> 2019-02-15  Michael Saboff  <msaboff@apple.com>
> 
>         RELEASE_ASSERT at com.apple.JavaScriptCore: JSC::jsSubstringOfResolved
>diff --git a/JSTests/microbenchmarks/mod-untyped.js b/JSTests/microbenchmarks/mod-untyped.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..800864d03d1133012e697418f5ee12d17ca5249f
>--- /dev/null
>+++ b/JSTests/microbenchmarks/mod-untyped.js
>@@ -0,0 +1,39 @@
>+let assert = {
>+    sameValue: function(i, e, m) {
>+        if (i !== e)
>+            throw new Error(m);
>+    }
>+}
>+
>+function untypedMod(x, y) {
>+    return x % y;
>+}
>+noInline(untypedMod);
>+
>+let o =  {valueOf: () => 10};
>+
>+for (let i = 0; i < 100000; i++) {
>+    let r = untypedMod(30, o);
>+    assert.sameValue(r, 0, 30 + " % {valueOf: () => 10} = " + r);
>+}
>+
>+o2 =  {valueOf: () => 10000};
>+
>+for (let i = 0; i < 100000; i++) {
>+    let r = untypedMod(o2, o);
>+    assert.sameValue(r, 0, "{valueOf: () => 10000} % {valueOf: () => 10}  = " + r);
>+}
>+
>+o = Object(10);
>+let r = untypedMod(30, o);
>+assert.sameValue(r, 0, 30 + " % Object(10) = " + r);
>+
>+o2 = Object(3240);
>+r = untypedMod(o2, o);
>+assert.sameValue(r, 0, "Object(3240) % Object(10) = " + r);
>+
>+for (let i = 0; i < 100000; i++) {
>+    let r = untypedMod("9", "8");
>+    assert.sameValue(r, 1, "9 % 8 = " + r);
>+}
>+
>diff --git a/JSTests/stress/big-int-mod-osr.js b/JSTests/stress/big-int-mod-osr.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..8c109052c2b9c74e5a4476a0c861d570122b1008
>--- /dev/null
>+++ b/JSTests/stress/big-int-mod-osr.js
>@@ -0,0 +1,30 @@
>+//@ runBigIntEnabled
>+
>+let assert = {
>+    sameValue: function(i, e, m) {
>+        if (i !== e)
>+            throw new Error(m);
>+    }
>+}
>+
>+function bigIntMod(x, y) {
>+    return x % y;
>+}
>+noInline(bigIntMod);
>+
>+for (let i = 0; i < 10000; i++) {
>+    let r = bigIntMod(3n, 10n);
>+    assert.sameValue(r, 3n, 3n + " % " + 10n + " = " + r);
>+}
>+
>+let r = bigIntMod(3, 10);
>+assert.sameValue(r, 3, 3 + " % " + 10 + " = " + r);
>+
>+for (let i = 0; i < 10000; i++) {
>+    let r = bigIntMod(3n, 10n);
>+    assert.sameValue(r, 3n, 3n + " % " + 10n + " = " + r);
>+}
>+
>+r = bigIntMod("3", "10");
>+assert.sameValue(r, 3, 3 + " % " + 10 + " = " + r);
>+
>diff --git a/PerformanceTests/BigIntBench/big-int-simple-mod.js b/PerformanceTests/BigIntBench/big-int-simple-mod.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..a50dc6ff372d153523a9c873cea55d32304e1133
>--- /dev/null
>+++ b/PerformanceTests/BigIntBench/big-int-simple-mod.js
>@@ -0,0 +1,15 @@
>+function bigInt(a, b) {
>+    let c = a % b;
>+    return a + c % b;
>+}
>+noInline(bigInt);
>+
>+for (let i = 0; i < 100000; i++) {
>+    bigInt(0b1111n, 0b1010n);
>+}
>+
>+let out;
>+for (let i = 0; i < 100000; i++) {
>+    out = bigInt(0xffffffffffffffffffn, 0xaaffffffffffffffffffn);
>+}
>+
>diff --git a/PerformanceTests/ChangeLog b/PerformanceTests/ChangeLog
>index d6d63135d0f5d951ad9f968eb3991e3ca70f4a1b..b214e321e8daeb8d711513efa242ae72eb90a337 100644
>--- a/PerformanceTests/ChangeLog
>+++ b/PerformanceTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2019-02-17  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [BigInt] Add ValueMod into DFG
>+        https://bugs.webkit.org/show_bug.cgi?id=186174
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * BigIntBench/big-int-simple-mod.js: Added.
>+
> 2019-02-08  Saam barati  <sbarati@apple.com>
> 
>         Update JetStream2 CLI to not print "ms" since we now just print the score values

Actions: View | Formatted Diff | Diff

Attachments on bug 186174: 357312 | 357558 | 357559 | 357630 | 357669 | 357671 | 358259 | 358260 | 359609 | 359875 | 359877 | 359973 | 360005 | 360926 | 362254 | 364011 | 367063 | 368939 | 368952 | 369347 | 369361 | 369381 | 369390 | 369393