Changeset 196726 in webkit

Timestamp:

Feb 17, 2016 3:35:11 PM (8 years ago)

Author:

commit-queue@webkit.org

Message:

[JSC] Remove the overflow check on ArithAbs when possible
​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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGFixupPhase.cpp (modified) (1 diff)
• dfg/DFGIntegerRangeOptimizationPhase.cpp (modified) (2 diffs)
• dfg/DFGNode.h (modified) (2 diffs)
• dfg/DFGSpeculativeJIT64.cpp (modified) (1 diff)
• ftl/FTLLowerDFGToLLVM.cpp (modified) (1 diff)
• tests/stress/arith-abs-integer-range-optimization.js (added)
• tests/stress/arith-abs-with-bitwise-or-zero.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -335 +335 @@
             if (m_graph.unaryArithShouldSpeculateInt32(node, FixupPass)) {
                 fixIntOrBooleanEdge(node->child1());
+                if (bytecodeCanTruncateInteger(node->arithNodeFlags()))
+                    node->setArithMode(Arith::Unchecked);
+                else
+                    node->setArithMode(Arith::CheckOverflow);
                 break;
             }

trunk/Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp

@@ -1204 +1204 @@
                 // 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))
@@ -1308 +1345 @@
             setRelationship(Relationship::safeCreate(node->child1().node(), node->child2().node(), Relationship::LessThan));
             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;
         }

trunk/Source/JavaScriptCore/dfg/DFGNode.h

@@ -650 +650 @@
         child2() = Edge();
         m_op = ArithSqrt;
+    }
+
+    void convertToArithNegate()
+    {
+        ASSERT(m_op == ArithAbs && child1().useKind() == Int32Use);
+        m_op = ArithNegate;
     }
     
@@ -1679 +1685 @@
     {
         switch (op()) {
+        case ArithAbs:
         case ArithAdd:
         case ArithSub:

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

@@ -2303 +2303 @@
             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;

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

@@ -2125 +2125 @@
         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;