[JSC] Clean up the abstract interpreter for cos/sin/sqrt/fround/log
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Aug 2016 23:02:52 +0000 (23:02 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Aug 2016 23:02:52 +0000 (23:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161181

Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-08-25
Reviewed by Geoffrey Garen.

JSTests:

Extend the tests to constants.
Add no-argument cases where needed.

* stress/arith-cos-on-various-types.js:
* stress/arith-fround-on-various-types.js:
* stress/arith-log-on-various-types.js:
* stress/arith-sin-on-various-types.js:
* stress/arith-sqrt-on-various-types.js:

Source/JavaScriptCore:

All the nodes are doing the exact same thing with a single
difference: how to process constants. I made that into a separate
function called from each node.

I also generalized the constant-to-number code of DoubleRep
to make it available for all those nodes.

* dfg/DFGAbstractInterpreter.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects):
* runtime/JSCJSValue.cpp:
(JSC::JSValue::toNumberFromPrimitive):
* runtime/JSCJSValue.h:

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

JSTests/ChangeLog
JSTests/stress/arith-cos-on-various-types.js
JSTests/stress/arith-fround-on-various-types.js
JSTests/stress/arith-log-on-various-types.js
JSTests/stress/arith-sin-on-various-types.js
JSTests/stress/arith-sqrt-on-various-types.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/runtime/JSCJSValue.cpp
Source/JavaScriptCore/runtime/JSCJSValue.h

index 7fba3fb..d463327 100644 (file)
@@ -1,3 +1,19 @@
+2016-08-25  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Clean up the abstract interpreter for cos/sin/sqrt/fround/log
+        https://bugs.webkit.org/show_bug.cgi?id=161181
+
+        Reviewed by Geoffrey Garen.
+
+        Extend the tests to constants.
+        Add no-argument cases where needed.
+
+        * stress/arith-cos-on-various-types.js:
+        * stress/arith-fround-on-various-types.js:
+        * stress/arith-log-on-various-types.js:
+        * stress/arith-sin-on-various-types.js:
+        * stress/arith-sqrt-on-various-types.js:
+
 2016-08-25  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [DFG][FTL] Implement ES6 Generators in DFG / FTL
index ccae892..7b0f053 100644 (file)
@@ -34,6 +34,26 @@ function isIdentical(result, expected)
 }
 
 
+// Test Math.cos() without arguments.
+function opaqueCosNoArgument() {
+    return Math.cos();
+}
+noInline(opaqueCosNoArgument);
+noOSRExitFuzzing(opaqueCosNoArgument);
+
+function testNoArgument() {
+    for (let i = 0; i < 1e4; ++i) {
+        let output = opaqueCosNoArgument();
+        if (output === output) {
+            throw "Failed opaqueCosNoArgument";
+        }
+    }
+    if (numberOfDFGCompiles(opaqueCosNoArgument) > 1)
+        throw "The call without arguments should never exit.";
+}
+testNoArgument();
+
+
 // Test Math.cos() with a very polymorphic input. All test cases are seen at each iteration.
 function opaqueAllTypesCos(argument) {
     return Math.cos(argument);
@@ -78,6 +98,29 @@ function testSingleTypeCall() {
 testSingleTypeCall();
 
 
+// Test Math.cos() on constants
+function testConstant() {
+    for (let testCaseInput of validInputTestCases) {
+        eval(`
+            function opaqueCosOnConstant() {
+                return Math.cos(${testCaseInput[0]});
+            }
+            noInline(opaqueCosOnConstant);
+            noOSRExitFuzzing(opaqueCosOnConstant);
+
+            for (let i = 0; i < 1e4; ++i) {
+                if (!isIdentical(opaqueCosOnConstant(), ${testCaseInput[1]})) {
+                    throw "Failed testConstant()";
+                }
+            }
+            if (numberOfDFGCompiles(opaqueCosOnConstant) > 1)
+                throw "We should have compiled a single cos for the expected type.";
+        `);
+    }
+}
+testConstant();
+
+
 // Verify we call valueOf() exactly once per call.
 function opaqueCosForSideEffects(argument) {
     return Math.cos(argument);
index 3e17ab2..a3d7fae 100644 (file)
@@ -100,6 +100,29 @@ function testSingleTypeCall() {
 testSingleTypeCall();
 
 
+// Test Math.fround() on constants
+function testConstant() {
+    for (let testCaseInput of validInputTestCases) {
+        eval(`
+            function opaqueFroundOnConstant() {
+                return Math.fround(${testCaseInput[0]});
+            }
+            noInline(opaqueFroundOnConstant);
+            noOSRExitFuzzing(opaqueFroundOnConstant);
+
+            for (let i = 0; i < 1e4; ++i) {
+                if (!isIdentical(opaqueFroundOnConstant(), ${testCaseInput[1]})) {
+                    throw "Failed testConstant()";
+                }
+            }
+            if (numberOfDFGCompiles(opaqueFroundOnConstant) > 1)
+                throw "We should have compiled a single fround for the expected type.";
+        `);
+    }
+}
+testConstant();
+
+
 // Verify we call valueOf() exactly once per call.
 function opaqueFroundForSideEffects(argument) {
     return Math.fround(argument);
index 55e8210..ca6e3db 100644 (file)
@@ -37,6 +37,26 @@ function isIdentical(result, expected)
 }
 
 
+// Test Math.log() without arguments.
+function opaqueLogNoArgument() {
+    return Math.log();
+}
+noInline(opaqueLogNoArgument);
+noOSRExitFuzzing(opaqueLogNoArgument);
+
+function testNoArgument() {
+    for (let i = 0; i < 1e4; ++i) {
+        let output = opaqueLogNoArgument();
+        if (output === output) {
+            throw "Failed opaqueLogNoArgument";
+        }
+    }
+    if (numberOfDFGCompiles(opaqueLogNoArgument) > 1)
+        throw "The call without arguments should never exit.";
+}
+testNoArgument();
+
+
 // Test Math.log() with a very polymorphic input. All test cases are seen at each iteration.
 function opaqueAllTypesLog(argument) {
     return Math.log(argument);
@@ -81,6 +101,29 @@ function testSingleTypeCall() {
 testSingleTypeCall();
 
 
+// Test Math.log() on constants
+function testConstant() {
+    for (let testCaseInput of validInputTestCases) {
+        eval(`
+            function opaqueLogOnConstant() {
+                return Math.log(${testCaseInput[0]});
+            }
+            noInline(opaqueLogOnConstant);
+            noOSRExitFuzzing(opaqueLogOnConstant);
+
+            for (let i = 0; i < 1e4; ++i) {
+                if (!isIdentical(opaqueLogOnConstant(), ${testCaseInput[1]})) {
+                    throw "Failed testConstant()";
+                }
+            }
+            if (numberOfDFGCompiles(opaqueLogOnConstant) > 1)
+                throw "We should have compiled a single log for the expected type.";
+        `);
+    }
+}
+testConstant();
+
+
 // Verify we call valueOf() exactly once per call.
 function opaqueLogForSideEffects(argument) {
     return Math.log(argument);
index 0429f46..54b8a35 100644 (file)
@@ -34,6 +34,26 @@ function isIdentical(result, expected)
 }
 
 
+// Test Math.sin() without arguments.
+function opaqueSinNoArgument() {
+    return Math.sin();
+}
+noInline(opaqueSinNoArgument);
+noOSRExitFuzzing(opaqueSinNoArgument);
+
+function testNoArgument() {
+    for (let i = 0; i < 1e4; ++i) {
+        let output = opaqueSinNoArgument();
+        if (output === output) {
+            throw "Failed opaqueSinNoArgument";
+        }
+    }
+    if (numberOfDFGCompiles(opaqueSinNoArgument) > 1)
+        throw "The call without arguments should never exit.";
+}
+testNoArgument();
+
+
 // Test Math.sin() with a very polymorphic input. All test cases are seen at each iteration.
 function opaqueAllTypesSin(argument) {
     return Math.sin(argument);
@@ -78,6 +98,29 @@ function testSingleTypeCall() {
 testSingleTypeCall();
 
 
+// Test Math.sin() on constants
+function testConstant() {
+    for (let testCaseInput of validInputTestCases) {
+        eval(`
+            function opaqueSinOnConstant() {
+                return Math.sin(${testCaseInput[0]});
+            }
+            noInline(opaqueSinOnConstant);
+            noOSRExitFuzzing(opaqueSinOnConstant);
+
+            for (let i = 0; i < 1e4; ++i) {
+                if (!isIdentical(opaqueSinOnConstant(), ${testCaseInput[1]})) {
+                    throw "Failed testConstant()";
+                }
+            }
+            if (numberOfDFGCompiles(opaqueSinOnConstant) > 1)
+                throw "We should have compiled a single sin for the expected type.";
+        `);
+    }
+}
+testConstant();
+
+
 // Verify we call valueOf() exactly once per call.
 function opaqueSinForSideEffects(argument) {
     return Math.sin(argument);
index 0060a0a..e3bb078 100644 (file)
@@ -32,6 +32,26 @@ function isIdentical(result, expected)
 }
 
 
+// Test Math.sqrt() without arguments.
+function opaqueSqrtNoArgument() {
+    return Math.sqrt();
+}
+noInline(opaqueSqrtNoArgument);
+noOSRExitFuzzing(opaqueSqrtNoArgument);
+
+function testNoArgument() {
+    for (let i = 0; i < 1e4; ++i) {
+        let output = opaqueSqrtNoArgument();
+        if (output === output) {
+            throw "Failed opaqueSqrtNoArgument";
+        }
+    }
+    if (numberOfDFGCompiles(opaqueSqrtNoArgument) > 1)
+        throw "The call without arguments should never exit.";
+}
+testNoArgument();
+
+
 // Test Math.sqrt() with a very polymorphic input. All test cases are seen at each iteration.
 function opaqueAllTypesSqrt(argument) {
     return Math.sqrt(argument);
@@ -76,6 +96,29 @@ function testSingleTypeCall() {
 testSingleTypeCall();
 
 
+// Test Math.sqrt() on constants
+function testConstant() {
+    for (let testCaseInput of validInputTestCases) {
+        eval(`
+            function opaqueSqrtOnConstant() {
+                return Math.sqrt(${testCaseInput[0]});
+            }
+            noInline(opaqueSqrtOnConstant);
+            noOSRExitFuzzing(opaqueSqrtOnConstant);
+
+            for (let i = 0; i < 1e4; ++i) {
+                if (!isIdentical(opaqueSqrtOnConstant(), ${testCaseInput[1]})) {
+                    throw "Failed testConstant()";
+                }
+            }
+            if (numberOfDFGCompiles(opaqueSqrtOnConstant) > 1)
+                throw "We should have compiled a single sqrt for the expected type.";
+        `);
+    }
+}
+testConstant();
+
+
 // Verify we call valueOf() exactly once per call.
 function opaqueSqrtForSideEffects(argument) {
     return Math.sqrt(argument);
index 9a7f28d..1943e7c 100644 (file)
@@ -1,3 +1,25 @@
+2016-08-25  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Clean up the abstract interpreter for cos/sin/sqrt/fround/log
+        https://bugs.webkit.org/show_bug.cgi?id=161181
+
+        Reviewed by Geoffrey Garen.
+
+        All the nodes are doing the exact same thing with a single
+        difference: how to process constants. I made that into a separate
+        function called from each node.
+
+        I also generalized the constant-to-number code of DoubleRep
+        to make it available for all those nodes.
+
+        * dfg/DFGAbstractInterpreter.h:
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects):
+        * runtime/JSCJSValue.cpp:
+        (JSC::JSValue::toNumberFromPrimitive):
+        * runtime/JSCJSValue.h:
+
 2016-08-25  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [DFG][FTL] Implement ES6 Generators in DFG / FTL
index 0178508..1c7e99c 100644 (file)
@@ -186,6 +186,7 @@ private:
     
     void verifyEdge(Node*, Edge);
     void verifyEdges(Node*);
+    void executeDoubleUnaryOpEffects(Node*, double(*equivalentFunction)(double));
     
     CodeBlock* m_codeBlock;
     Graph& m_graph;
index 3297e37..38df41c 100644 (file)
@@ -433,23 +433,9 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         
     case DoubleRep: {
         JSValue child = forNode(node->child1()).value();
-        if (child) {
-            if (child.isNumber()) {
-                setConstant(node, jsDoubleNumber(child.asNumber()));
-                break;
-            }
-            if (child.isUndefined()) {
-                setConstant(node, jsDoubleNumber(PNaN));
-                break;
-            }
-            if (child.isNull() || child.isFalse()) {
-                setConstant(node, jsDoubleNumber(0));
-                break;
-            }
-            if (child.isTrue()) {
-                setConstant(node, jsDoubleNumber(1));
-                break;
-            }
+        if (Optional<double> number = child.toNumberFromPrimitive()) {
+            setConstant(node, jsDoubleNumber(*number));
+            break;
         }
 
         SpeculatedType type = forNode(node->child1()).m_type;
@@ -955,70 +941,25 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
     }
             
-    case ArithSqrt: {
-        JSValue child = forNode(node->child1()).value();
-        if (child && child.isNumber()) {
-            setConstant(node, jsDoubleNumber(sqrt(child.asNumber())));
-            break;
-        }
-        SpeculatedType sqrtType = SpecFullNumber;
-        if (node->child1().useKind() == DoubleRepUse)
-            sqrtType = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
-        forNode(node).setType(sqrtType);
+    case ArithSqrt:
+        executeDoubleUnaryOpEffects(node, sqrt);
         break;
-    }
-        
-    case ArithFRound: {
-        JSValue child = forNode(node->child1()).value();
-        if (child && child.isNumber()) {
-            setConstant(node, jsDoubleNumber(static_cast<float>(child.asNumber())));
-            break;
-        }
-        SpeculatedType froundType = SpecFullNumber;
-        if (node->child1().useKind() == DoubleRepUse)
-            froundType = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
-        forNode(node).setType(froundType);
+
+    case ArithFRound:
+        executeDoubleUnaryOpEffects(node, [](double value) -> double { return static_cast<float>(value); });
         break;
-    }
         
-    case ArithSin: {
-        JSValue child = forNode(node->child1()).value();
-        if (child && child.isNumber()) {
-            setConstant(node, jsDoubleNumber(sin(child.asNumber())));
-            break;
-        }
-        SpeculatedType sinType = SpecFullNumber;
-        if (node->child1().useKind() == DoubleRepUse)
-            sinType = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
-        forNode(node).setType(sinType);
+    case ArithSin:
+        executeDoubleUnaryOpEffects(node, sin);
         break;
-    }
     
-    case ArithCos: {
-        JSValue child = forNode(node->child1()).value();
-        if (child && child.isNumber()) {
-            setConstant(node, jsDoubleNumber(cos(child.asNumber())));
-            break;
-        }
-        SpeculatedType cosType = SpecFullNumber;
-        if (node->child1().useKind() == DoubleRepUse)
-            cosType = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
-        forNode(node).setType(cosType);
+    case ArithCos:
+        executeDoubleUnaryOpEffects(node, cos);
         break;
-    }
 
-    case ArithLog: {
-        JSValue child = forNode(node->child1()).value();
-        if (child && child.isNumber()) {
-            setConstant(node, jsDoubleNumber(log(child.asNumber())));
-            break;
-        }
-        SpeculatedType logType = SpecFullNumber;
-        if (node->child1().useKind() == DoubleRepUse)
-            logType = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
-        forNode(node).setType(logType);
+    case ArithLog:
+        executeDoubleUnaryOpEffects(node, log);
         break;
-    }
             
     case LogicalNot: {
         switch (booleanResult(node, forNode(node->child1()))) {
@@ -3102,6 +3043,20 @@ FiltrationResult AbstractInterpreter<AbstractStateType>::filterByValue(
     return Contradiction;
 }
 
+template<typename AbstractStateType>
+void AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects(Node* node, double(*equivalentFunction)(double))
+{
+    JSValue child = forNode(node->child1()).value();
+    if (Optional<double> number = child.toNumberFromPrimitive()) {
+        setConstant(node, jsDoubleNumber(equivalentFunction(*number)));
+        return;
+    }
+    SpeculatedType type = SpecFullNumber;
+    if (node->child1().useKind() == DoubleRepUse)
+        type = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
+    forNode(node).setType(type);
+}
+
 } } // namespace JSC::DFG
 
 #endif // ENABLE(DFG_JIT)
index d766e42..0a77b95 100644 (file)
@@ -77,6 +77,21 @@ double JSValue::toNumberSlowCase(ExecState* exec) const
     return isUndefined() ? PNaN : 0; // null and false both convert to 0.
 }
 
+Optional<double> JSValue::toNumberFromPrimitive() const
+{
+    if (isEmpty())
+        return Nullopt;
+    if (isNumber())
+        return asNumber();
+    if (isBoolean())
+        return asBoolean();
+    if (isUndefined())
+        return PNaN;
+    if (isNull())
+        return 0;
+    return Nullopt;
+}
+
 JSObject* JSValue::toObjectSlowCase(ExecState* exec, JSGlobalObject* globalObject) const
 {
     ASSERT(!isCell());
index 40ee7fa..a4f6f68 100644 (file)
@@ -261,6 +261,10 @@ public:
     // toNumber conversion is expected to be side effect free if an exception has
     // been set in the ExecState already.
     double toNumber(ExecState*) const;
+
+    // toNumber conversion if it can be done without side effects.
+    Optional<double> toNumberFromPrimitive() const;
+
     JSString* toString(ExecState*) const; // On exception, this returns the empty string.
     JSString* toStringOrNull(ExecState*) const; // On exception, this returns null, to make exception checks faster.
     Identifier toPropertyKey(ExecState*) const;