[JSC] Remove the overflow check on ArithAbs when possible
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Feb 2016 23:35:11 +0000 (23:35 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Feb 2016 23:35:11 +0000 (23:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154325

Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-02-17
Reviewed by Filip Pizlo.

This patch adds support for ArithMode for ArithAbs.

It is useful for kraken tests where Math.abs() is used
on values for which the range is known.

For example, imaging-gaussian-blur has two Math.abs() with
integers that are always in a small range around zero.
The IntegerRangeOptimizationPhase detects the range correctly
so we can just update the ArithMode depending on the input.

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGIntegerRangeOptimizationPhase.cpp:
* dfg/DFGNode.h:
(JSC::DFG::Node::convertToArithNegate):
(JSC::DFG::Node::hasArithMode):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compileArithAbs):
* tests/stress/arith-abs-integer-range-optimization.js: Added.
(negativeRange):
(negativeRangeIncludingZero):
(negativeRangeWithOverflow):
(positiveRange):
(positiveRangeIncludingZero):
(rangeWithoutOverflow):
* tests/stress/arith-abs-with-bitwise-or-zero.js: Added.
(opaqueAbs):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
Source/JavaScriptCore/tests/stress/arith-abs-integer-range-optimization.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/arith-abs-with-bitwise-or-zero.js [new file with mode: 0644]

index 90d7565..d55a7ff 100644 (file)
@@ -1,3 +1,40 @@
+2016-02-17  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Remove the overflow check on ArithAbs when possible
+        https://bugs.webkit.org/show_bug.cgi?id=154325
+
+        Reviewed by Filip Pizlo.
+
+        This patch adds support for ArithMode for ArithAbs.
+
+        It is useful for kraken tests where Math.abs() is used
+        on values for which the range is known.
+
+        For example, imaging-gaussian-blur has two Math.abs() with
+        integers that are always in a small range around zero.
+        The IntegerRangeOptimizationPhase detects the range correctly
+        so we can just update the ArithMode depending on the input.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGIntegerRangeOptimizationPhase.cpp:
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::convertToArithNegate):
+        (JSC::DFG::Node::hasArithMode):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::DFG::LowerDFGToLLVM::compileArithAbs):
+        * tests/stress/arith-abs-integer-range-optimization.js: Added.
+        (negativeRange):
+        (negativeRangeIncludingZero):
+        (negativeRangeWithOverflow):
+        (positiveRange):
+        (positiveRangeIncludingZero):
+        (rangeWithoutOverflow):
+        * tests/stress/arith-abs-with-bitwise-or-zero.js: Added.
+        (opaqueAbs):
+
 2016-02-17  Chris Dumez  <cdumez@apple.com>
 
         SES selftest page crashes on nightly r196694
index b461e7f..6a4eb13 100644 (file)
@@ -334,6 +334,10 @@ private:
         case ArithAbs: {
             if (m_graph.unaryArithShouldSpeculateInt32(node, FixupPass)) {
                 fixIntOrBooleanEdge(node->child1());
+                if (bytecodeCanTruncateInteger(node->arithNodeFlags()))
+                    node->setArithMode(Arith::Unchecked);
+                else
+                    node->setArithMode(Arith::CheckOverflow);
                 break;
             }
             fixDoubleOrBooleanEdge(node->child1());
index 7df9004..02aa134 100644 (file)
@@ -1203,6 +1203,43 @@ public:
                 // optimize by using the relationships before the operation, but we need to
                 // call executeNode() before we optimize.
                 switch (node->op()) {
+                case ArithAbs: {
+                    if (node->child1().useKind() != Int32Use)
+                        break;
+
+                    auto iter = m_relationships.find(node->child1().node());
+                    if (iter == m_relationships.end())
+                        break;
+
+                    int minValue = std::numeric_limits<int>::min();
+                    int maxValue = std::numeric_limits<int>::max();
+                    for (Relationship relationship : iter->value) {
+                        minValue = std::max(minValue, relationship.minValueOfLeft());
+                        maxValue = std::min(maxValue, relationship.maxValueOfLeft());
+                    }
+
+                    executeNode(block->at(nodeIndex));
+
+                    if (minValue >= 0) {
+                        node->convertToIdentityOn(node->child1().node());
+                        changed = true;
+                        break;
+                    }
+                    if (maxValue <= 0) {
+                        node->convertToArithNegate();
+                        if (minValue > std::numeric_limits<int>::min())
+                            node->setArithMode(Arith::Unchecked);
+                        changed = true;
+                        break;
+                    }
+                    if (minValue > std::numeric_limits<int>::min()) {
+                        node->setArithMode(Arith::Unchecked);
+                        changed = true;
+                        break;
+                    }
+
+                    break;
+                }
                 case ArithAdd: {
                     if (!node->isBinaryUseKind(Int32Use))
                         break;
@@ -1309,6 +1346,13 @@ private:
             setRelationship(Relationship::safeCreate(node->child1().node(), m_zero, Relationship::GreaterThan, -1));
             break;
         }
+
+        case ArithAbs: {
+            if (node->child1().useKind() != Int32Use)
+                break;
+            setRelationship(Relationship(node, m_zero, Relationship::GreaterThan, -1));
+            break;
+        }
             
         case ArithAdd: {
             // We're only interested in int32 additions and we currently only know how to
index 12870d9..7612896 100644 (file)
@@ -650,6 +650,12 @@ struct Node {
         child2() = Edge();
         m_op = ArithSqrt;
     }
+
+    void convertToArithNegate()
+    {
+        ASSERT(m_op == ArithAbs && child1().useKind() == Int32Use);
+        m_op = ArithNegate;
+    }
     
     JSValue asJSValue()
     {
@@ -1678,6 +1684,7 @@ struct Node {
     bool hasArithMode()
     {
         switch (op()) {
+        case ArithAbs:
         case ArithAdd:
         case ArithSub:
         case ArithNegate:
index 140311e..e564ccb 100644 (file)
@@ -2302,7 +2302,8 @@ void SpeculativeJIT::compile(Node* node)
             m_jit.rshift32(result.gpr(), MacroAssembler::TrustedImm32(31), scratch.gpr());
             m_jit.add32(scratch.gpr(), result.gpr());
             m_jit.xor32(scratch.gpr(), result.gpr());
-            speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branch32(MacroAssembler::Equal, result.gpr(), MacroAssembler::TrustedImm32(1 << 31)));
+            if (shouldCheckOverflow(node->arithMode()))
+                speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branch32(MacroAssembler::Equal, result.gpr(), MacroAssembler::TrustedImm32(1 << 31)));
             int32Result(result.gpr(), node);
             break;
         }
index c31399e..2ff3c38 100644 (file)
@@ -2124,12 +2124,13 @@ private:
         switch (m_node->child1().useKind()) {
         case Int32Use: {
             LValue value = lowInt32(m_node->child1());
-            
+
             LValue mask = m_out.aShr(value, m_out.constInt32(31));
             LValue result = m_out.bitXor(mask, m_out.add(mask, value));
-            
-            speculate(Overflow, noValue(), 0, m_out.equal(result, m_out.constInt32(1 << 31)));
-            
+
+            if (shouldCheckOverflow(m_node->arithMode()))
+                speculate(Overflow, noValue(), 0, m_out.equal(result, m_out.constInt32(1 << 31)));
+
             setInt32(result);
             break;
         }
diff --git a/Source/JavaScriptCore/tests/stress/arith-abs-integer-range-optimization.js b/Source/JavaScriptCore/tests/stress/arith-abs-integer-range-optimization.js
new file mode 100644 (file)
index 0000000..5be96f7
--- /dev/null
@@ -0,0 +1,139 @@
+function negativeRange(results)
+{
+    for (var i = -1; i > -10; --i) {
+        results[Math.abs(i)] = i;
+    }
+}
+noInline(negativeRange);
+
+for (var i = 0; i < 1e4; ++i) {
+    var results = [];
+    negativeRange(results);
+    if (results.length != 10)
+        throw "Wrong result length: " + results.length;
+    for (var j = 0; j < 10; ++j) {
+        if (j < 1) {
+            if (results[j] !== undefined)
+                throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+            continue;
+        }
+        if (results[j] !== -j)
+            throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+    }
+}
+
+function negativeRangeIncludingZero(results)
+{
+    for (var i = 0; i > -10; --i) {
+        results[Math.abs(i)] = i;
+    }
+}
+noInline(negativeRangeIncludingZero);
+
+for (var i = 0; i < 1e4; ++i) {
+    var results = [];
+    negativeRangeIncludingZero(results);
+    if (results.length != 10)
+        throw "Wrong result length: " + results.length;
+    for (var j = 0; j < 10; ++j) {
+        if (results[j] !== -j)
+            throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+    }
+}
+
+function negativeRangeWithOverflow(results, limit)
+{
+    var i = -2147483648 + 10;
+    do {
+        results.push(Math.abs(i));
+        --i;
+    } while (i !== limit);
+}
+noInline(negativeRangeWithOverflow);
+
+// First, we warm up without overflow.
+for (var i = 0; i < 1e4; ++i) {
+    var results = [];
+    negativeRangeWithOverflow(results, -2147483647);
+    if (results.length != 9)
+        throw "Wrong result length: " + results.length;
+    for (var j = 0; j < 9; ++j) {
+        if (results[j] !== 2147483638 + j)
+            throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+    }
+}
+
+// Then we overflow.
+for (var i = 0; i < 1e4; ++i) {
+    var results = [];
+    negativeRangeWithOverflow(results, -2147483648);
+    if (results.length != 10)
+        throw "Wrong result length: " + results.length;
+    for (var j = 0; j < 10; ++j) {
+        if (results[j] !== 2147483638 + j)
+            throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+    }
+}
+
+function positiveRange(results)
+{
+    for (var i = 1; i < 10; ++i) {
+        results[Math.abs(i)] = i;
+    }
+}
+noInline(positiveRange);
+
+for (var i = 0; i < 1e4; ++i) {
+    var results = [];
+    positiveRange(results);
+    if (results.length != 10)
+        throw "Wrong result length: " + results.length;
+    for (var j = 0; j < 10; ++j) {
+        if (j < 1) {
+            if (results[j] !== undefined)
+                throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+            continue;
+        }
+        if (results[j] !== j)
+            throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+    }
+}
+
+function positiveRangeIncludingZero(results)
+{
+    for (var i = 0; i < 10; ++i) {
+        results[Math.abs(i)] = i;
+    }
+}
+noInline(positiveRangeIncludingZero);
+
+for (var i = 0; i < 1e4; ++i) {
+    var results = [];
+    positiveRangeIncludingZero(results);
+    if (results.length != 10)
+        throw "Wrong result length: " + results.length;
+    for (var j = 0; j < 10; ++j) {
+        if (results[j] !== j)
+            throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+    }
+}
+
+function rangeWithoutOverflow(results)
+{
+    for (var i = -10; i < 10; ++i) {
+        results[i] = Math.abs(i);
+    }
+}
+noInline(rangeWithoutOverflow);
+
+for (var i = 0; i < 1e4; ++i) {
+    var results = [];
+    rangeWithoutOverflow(results);
+    if (results.length != 10)
+        throw "Wrong result length: " + results.length;
+    for (var j = -10; j < 10; ++j) {
+        var expected = (j < 0) ? -j : j;
+        if (results[j] !== expected)
+            throw "Wrong result, results[j] = " + results[j] + " at j = " + j;
+    }
+}
diff --git a/Source/JavaScriptCore/tests/stress/arith-abs-with-bitwise-or-zero.js b/Source/JavaScriptCore/tests/stress/arith-abs-with-bitwise-or-zero.js
new file mode 100644 (file)
index 0000000..0f2450f
--- /dev/null
@@ -0,0 +1,54 @@
+function opaqueAbs(value)
+{
+    return Math.abs(value)|0;
+}
+noInline(opaqueAbs);
+
+for (let i = 0; i < 1e4; ++i) {
+    var positiveResult = opaqueAbs(i);
+    if (positiveResult !== i)
+        throw "Incorrect result at i = " + i + " result = " + positiveResult;
+    var negativeResult = opaqueAbs(-i);
+    if (negativeResult !== i)
+        throw "Incorrect result at -i = " + -i + " result = " + negativeResult;
+}
+
+
+var intMax = 2147483647;
+var intMin = 2147483647;
+
+var intMaxResult = opaqueAbs(intMax);
+if (intMaxResult !== intMax)
+    throw "Incorrect result at intMax result = " + intMaxResult;
+var intMaxResult = opaqueAbs(intMin);
+if (intMaxResult !== intMin)
+    throw "Incorrect result at intMax result = " + intMaxResult;
+
+// Numbers around IntMax/IntMin. Numbers outside the bounds are doubles and opaqueAbs()
+// has to OSR Exit to handle them correctly.
+for (let i = intMax - 1e4; i < intMax + 1e4; ++i) {
+    var positiveResult = opaqueAbs(i);
+    if (positiveResult !== (i|0))
+        throw "Incorrect result at i = " + i + " result = " + positiveResult;
+    var negativeResult = opaqueAbs(-i);
+    if (negativeResult !== (i|0))
+        throw "Incorrect result at -i = " + -i + " result = " + negativeResult;
+}
+
+// Edge cases and exits.
+if (opaqueAbs(NaN) !== 0)
+    throw "opaqueAbs(NaN) failed.";
+if (opaqueAbs(Infinity) !== 0)
+    throw "opaqueAbs(Infinity) failed.";
+if (opaqueAbs(-Infinity) !== 0)
+    throw "opaqueAbs(-Infinity) failed.";
+if (opaqueAbs(null) !== 0)
+    throw "opaqueAbs(null) failed.";
+if (opaqueAbs(undefined) !== 0)
+    throw "opaqueAbs(undefined) failed.";
+if (opaqueAbs(true) !== 1)
+    throw "opaqueAbs(true) failed.";
+if (opaqueAbs(false) !== 0)
+    throw "opaqueAbs(false) failed.";
+if (opaqueAbs({foo:"bar"}) !== 0)
+    throw "opaqueAbs({foo:'bar'}) failed.";