DFG abstract interpreter needs to properly model effects of some Math ops
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 18:37:55 +0000 (18:37 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 18:37:55 +0000 (18:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181886

Reviewed by Saam Barati.

JSTests:

New regression test.

* stress/arith-nodes-abstract-interpreter-untypeduse.js: Added.
(test):

Source/JavaScriptCore:

Reviewed the processing of the various ArithXXX and CompareXXX and found that
several nodes don't handle UntypedUse.  Added clobberWorld() for those cases.

* dfg/DFGAbstractInterpreter.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects):

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

JSTests/ChangeLog
JSTests/stress/arith-nodes-abstract-interpreter-untypeduse.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

index c12cab4..a68f1b2 100644 (file)
@@ -1,3 +1,15 @@
+2018-01-22  Michael Saboff  <msaboff@apple.com>
+
+        DFG abstract interpreter needs to properly model effects of some Math ops
+        https://bugs.webkit.org/show_bug.cgi?id=181886
+
+        Reviewed by Saam Barati.
+
+        New regression test.
+
+        * stress/arith-nodes-abstract-interpreter-untypeduse.js: Added.
+        (test):
+
 2018-01-20  Caio Lima  <ticaiolima@gmail.com>
 
         [JSC] NumberPrototype::extractRadixFromArgs incorrectly cast double to int32_t
diff --git a/JSTests/stress/arith-nodes-abstract-interpreter-untypeduse.js b/JSTests/stress/arith-nodes-abstract-interpreter-untypeduse.js
new file mode 100644 (file)
index 0000000..fb3715b
--- /dev/null
@@ -0,0 +1,373 @@
+// This test that the DFG Abstract interpreter properly handles UntypedUse for various ArithXXX and CompareXXX nodes.
+// This test should run without errors or crashing.
+
+let errors = 0;
+const smallValue = 2.3023e-320;
+
+function test(functionName, testFunc) {
+    try{
+        var ary_1 = [1.1, 2.2, 3.3];
+        var ary_2 = ary_1;
+        var f64_1 = new Float64Array(0x10);
+
+        for (var i = 0; i < 200000; i++)
+            testFunc(ary_1, f64_1, '1');
+
+        testFunc(ary_2, f64_1, { toString:()=>{ ary_2[0] = {}; return "3"}});
+        if (ary_2[2] != smallValue)
+            throw functionName + " returned the wrong value";
+    } catch(e) {
+        errors++;
+        print("Exception testing " + functionName + ": " + e);
+    };
+};
+
+for (let i = 0; i < 8; i++) {
+    test("Warmup", function(a, b, c){
+        a[0] = 1.1;
+        a[1] = 2.2;
+        var tmp = Math.abs(c);
+        b[0] = a[0];
+        a[2] = smallValue;
+    });
+}
+
+test("Unary -", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = -c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Unary +", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = +c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("+", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = a[1] + c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("-", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = a[1] - c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("*", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = a[1] * c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("/", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = a[1] / c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("%", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = a[1] % c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("**", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = c ** a[1];
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("prefix ++", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = ++c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("prefix --", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = --c;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("==", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = c == 7;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("<", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = c < 42;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("<=", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = c <= 7;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test(">", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = c > 3;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test(">=", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = c >= 12;
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.abs", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.abs(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.acos", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.acos(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.acosh", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.acosh(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.asin", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.asin(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.asinh", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.asinh(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.atan", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.atan(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.atan2", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.atan2(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.atanh", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.atanh(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.ceil", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.ceil(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.cos", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.cos(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.cosh", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.cosh(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.exp", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.exp(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.expm1", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.expm1(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.floor", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.floor(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.fround", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.fround(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.log", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.log(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.log1p", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.log1p(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.log10", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.log10(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.log2", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.log2(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.round", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.round(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.sin", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.sin(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.sign", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.sign(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.sinh", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.sinh(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.sqrt", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.sqrt(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.tan", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.tan(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.tanh", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.tanh(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+test("Math.trunc", function(a, b, c){
+    a[0] = 1.1;
+    a[1] = 2.2;
+    var tmp = Math.trunc(c);
+    b[0] = a[0];
+    a[2] = smallValue;
+});
+
+
+if (errors)
+    throw "Failed " + errors + " tests."
index f5945da..68b91ea 100644 (file)
@@ -1,3 +1,18 @@
+2018-01-22  Michael Saboff  <msaboff@apple.com>
+
+        DFG abstract interpreter needs to properly model effects of some Math ops
+        https://bugs.webkit.org/show_bug.cgi?id=181886
+
+        Reviewed by Saam Barati.
+
+        Reviewed the processing of the various ArithXXX and CompareXXX and found that
+        several nodes don't handle UntypedUse.  Added clobberWorld() for those cases.
+
+        * dfg/DFGAbstractInterpreter.h:
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects):
+
 2018-01-21  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Add a new feature flag for EXTRA_ZOOM_MODE and reintroduce AdditionalFeatureDefines.h
index 1411825..1d672da 100644 (file)
@@ -188,7 +188,7 @@ private:
     
     void verifyEdge(Node*, Edge);
     void verifyEdges(Node*);
-    void executeDoubleUnaryOpEffects(Node*, double(*equivalentFunction)(double));
+    void executeDoubleUnaryOpEffects(Node*, unsigned clobberLimit, double(*equivalentFunction)(double));
     
     CodeBlock* m_codeBlock;
     Graph& m_graph;
index 64f3db9..4f274d8 100644 (file)
@@ -764,6 +764,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
             break;
         default:
             DFG_ASSERT(m_graph, node, node->child1().useKind() == UntypedUse);
+            clobberWorld(node->origin.semantic, clobberLimit);
             forNode(node).setType(SpecBytecodeNumber);
             break;
         }
@@ -978,6 +979,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
             break;
         default:
             DFG_ASSERT(m_graph, node, node->child1().useKind() == UntypedUse);
+            clobberWorld(node->origin.semantic, clobberLimit);
             forNode(node).setType(SpecFullNumber);
             break;
         }
@@ -1054,21 +1056,22 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
                 forNode(node).setType(typeOfDoubleRounding(forNode(node->child1()).m_type));
         } else {
             DFG_ASSERT(m_graph, node, node->child1().useKind() == UntypedUse);
+            clobberWorld(node->origin.semantic, clobberLimit);
             forNode(node).setType(SpecFullNumber);
         }
         break;
     }
             
     case ArithSqrt:
-        executeDoubleUnaryOpEffects(node, sqrt);
+        executeDoubleUnaryOpEffects(node, clobberLimit, sqrt);
         break;
 
     case ArithFRound:
-        executeDoubleUnaryOpEffects(node, [](double value) -> double { return static_cast<float>(value); });
+        executeDoubleUnaryOpEffects(node, clobberLimit, [](double value) -> double { return static_cast<float>(value); });
         break;
         
     case ArithUnary:
-        executeDoubleUnaryOpEffects(node, arithUnaryFunction(node->arithUnaryType()));
+        executeDoubleUnaryOpEffects(node, clobberLimit, arithUnaryFunction(node->arithUnaryType()));
         break;
             
     case LogicalNot: {
@@ -1607,7 +1610,9 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
                 break;
             }
         }
-        
+
+        if (node->child1().useKind() == UntypedUse || node->child2().useKind() == UntypedUse)
+            clobberWorld(node->origin.semantic, clobberLimit);
         forNode(node).setType(SpecBoolean);
         break;
     }
@@ -3573,7 +3578,7 @@ FiltrationResult AbstractInterpreter<AbstractStateType>::filterClassInfo(
 }
 
 template<typename AbstractStateType>
-void AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects(Node* node, double(*equivalentFunction)(double))
+void AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects(Node* node, unsigned clobberLimit, double(*equivalentFunction)(double))
 {
     JSValue child = forNode(node->child1()).value();
     if (std::optional<double> number = child.toNumberFromPrimitive()) {
@@ -3583,6 +3588,8 @@ void AbstractInterpreter<AbstractStateType>::executeDoubleUnaryOpEffects(Node* n
     SpeculatedType type = SpecFullNumber;
     if (node->child1().useKind() == DoubleRepUse)
         type = typeOfDoubleUnaryOp(forNode(node->child1()).m_type);
+    else
+        clobberWorld(node->origin.semantic, clobberLimit);
     forNode(node).setType(type);
 }