Title: [167416] trunk/Source/_javascript_Core
Revision
167416
Author
fpi...@apple.com
Date
2014-04-16 21:57:29 -0700 (Wed, 16 Apr 2014)

Log Message

Sink NaN sanitization to uses and remove it when it's unnecessary
https://bugs.webkit.org/show_bug.cgi?id=131419

Reviewed by Oliver Hunt.
        
This moves NaN purification to stores that could see an impure NaN.
        
5% speed-up on AsmBench, 50% speed-up on AsmBench/n-body. It is a regression on FloatMM
though, because of the other bug that causes that benchmark to box doubles in a loop.

* bytecode/SpeculatedType.h:
(JSC::isInt32SpeculationForArithmetic):
(JSC::isMachineIntSpeculationForArithmetic):
(JSC::isDoubleSpeculation):
(JSC::isDoubleSpeculationForArithmetic):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGAbstractValue.cpp:
(JSC::DFG::AbstractValue::fixTypeForRepresentation):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::injectTypeConversionsForEdge):
* dfg/DFGInPlaceAbstractState.cpp:
(JSC::DFG::InPlaceAbstractState::mergeStateAtTail):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileValueRep):
(JSC::DFG::SpeculativeJIT::compileGetByValOnFloatTypedArray):
* dfg/DFGUseKind.h:
(JSC::DFG::typeFilterFor):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileValueRep):
(JSC::FTL::LowerDFGToLLVM::compileGetByVal):
* runtime/PureNaN.h:
* tests/stress/float32-array-nan-inlined.js: Added.
(foo):
(test):
* tests/stress/float32-array-nan.js: Added.
(foo):
(test):
* tests/stress/float64-array-nan-inlined.js: Added.
(foo):
(isBigEndian):
(test):
* tests/stress/float64-array-nan.js: Added.
(foo):
(isBigEndian):
(test):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (167415 => 167416)


--- trunk/Source/_javascript_Core/ChangeLog	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-04-17 04:57:29 UTC (rev 167416)
@@ -1,3 +1,55 @@
+2014-04-16  Filip Pizlo  <fpi...@apple.com>
+
+        Sink NaN sanitization to uses and remove it when it's unnecessary
+        https://bugs.webkit.org/show_bug.cgi?id=131419
+
+        Reviewed by Oliver Hunt.
+        
+        This moves NaN purification to stores that could see an impure NaN.
+        
+        5% speed-up on AsmBench, 50% speed-up on AsmBench/n-body. It is a regression on FloatMM
+        though, because of the other bug that causes that benchmark to box doubles in a loop.
+
+        * bytecode/SpeculatedType.h:
+        (JSC::isInt32SpeculationForArithmetic):
+        (JSC::isMachineIntSpeculationForArithmetic):
+        (JSC::isDoubleSpeculation):
+        (JSC::isDoubleSpeculationForArithmetic):
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGAbstractValue.cpp:
+        (JSC::DFG::AbstractValue::fixTypeForRepresentation):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        (JSC::DFG::FixupPhase::injectTypeConversionsForEdge):
+        * dfg/DFGInPlaceAbstractState.cpp:
+        (JSC::DFG::InPlaceAbstractState::mergeStateAtTail):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::propagate):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileValueRep):
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnFloatTypedArray):
+        * dfg/DFGUseKind.h:
+        (JSC::DFG::typeFilterFor):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileValueRep):
+        (JSC::FTL::LowerDFGToLLVM::compileGetByVal):
+        * runtime/PureNaN.h:
+        * tests/stress/float32-array-nan-inlined.js: Added.
+        (foo):
+        (test):
+        * tests/stress/float32-array-nan.js: Added.
+        (foo):
+        (test):
+        * tests/stress/float64-array-nan-inlined.js: Added.
+        (foo):
+        (isBigEndian):
+        (test):
+        * tests/stress/float64-array-nan.js: Added.
+        (foo):
+        (isBigEndian):
+        (test):
+
 2014-04-16  Brent Fulgham  <bfulg...@apple.com>
 
         [Win] Unreviewed Windows gardening. Restrict our new 'isinf' check

Modified: trunk/Source/_javascript_Core/bytecode/SpeculatedType.h (167415 => 167416)


--- trunk/Source/_javascript_Core/bytecode/SpeculatedType.h	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/bytecode/SpeculatedType.h	2014-04-17 04:57:29 UTC (rev 167416)
@@ -72,7 +72,7 @@
 static const SpeculatedType SpecDoubleImpureNaN    = 0x04000000; // It's definitely a NaN that is unsafe to tag (i.e. impure).
 static const SpeculatedType SpecDoubleNaN          = 0x06000000; // It's definitely some kind of NaN.
 static const SpeculatedType SpecBytecodeDouble     = 0x03800000; // It's either a non-NaN or a NaN double, but it's definitely not impure NaN.
-static const SpeculatedType SpecDouble             = 0x07800000; // It's either a non-NaN or a NaN double.
+static const SpeculatedType SpecFullDouble         = 0x07800000; // It's either a non-NaN or a NaN double.
 static const SpeculatedType SpecBytecodeRealNumber = 0x01a00000; // It's either an Int32 or a DoubleReal.
 static const SpeculatedType SpecFullRealNumber     = 0x01e00000; // It's either an Int32 or a DoubleReal, or a Int52.
 static const SpeculatedType SpecBytecodeNumber     = 0x03a00000; // It's either an Int32 or a Double, and the Double cannot be an impure NaN.
@@ -251,7 +251,7 @@
 
 inline bool isInt32SpeculationForArithmetic(SpeculatedType value)
 {
-    return !(value & (SpecDouble | SpecInt52));
+    return !(value & (SpecFullDouble | SpecInt52));
 }
 
 inline bool isInt32SpeculationExpectingDefined(SpeculatedType value)
@@ -276,7 +276,7 @@
 
 inline bool isMachineIntSpeculationForArithmetic(SpeculatedType value)
 {
-    return !(value & SpecDouble);
+    return !(value & SpecFullDouble);
 }
 
 inline bool isInt52AsDoubleSpeculation(SpeculatedType value)
@@ -296,12 +296,12 @@
 
 inline bool isDoubleSpeculation(SpeculatedType value)
 {
-    return !!value && (value & SpecDouble) == value;
+    return !!value && (value & SpecFullDouble) == value;
 }
 
 inline bool isDoubleSpeculationForArithmetic(SpeculatedType value)
 {
-    return !!(value & SpecDouble);
+    return !!(value & SpecFullDouble);
 }
 
 inline bool isBytecodeRealNumberSpeculation(SpeculatedType value)

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (167415 => 167416)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2014-04-17 04:57:29 UTC (rev 167416)
@@ -340,7 +340,7 @@
             break;
         }
         
-        forNode(node).setType(forNode(node->child1()).m_type);
+        forNode(node).setType(forNode(node->child1()).m_type & ~SpecDoubleImpureNaN);
         forNode(node).fixTypeForRepresentation(node);
         break;
     }
@@ -1092,10 +1092,10 @@
                 forNode(node).setType(SpecInt52AsDouble);
             break;
         case Array::Float32Array:
-            forNode(node).setType(SpecBytecodeDouble);
+            forNode(node).setType(SpecFullDouble);
             break;
         case Array::Float64Array:
-            forNode(node).setType(SpecBytecodeDouble);
+            forNode(node).setType(SpecFullDouble);
             break;
         default:
             RELEASE_ASSERT_NOT_REACHED();

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp (167415 => 167416)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp	2014-04-17 04:57:29 UTC (rev 167416)
@@ -95,7 +95,7 @@
             m_type &= ~SpecMachineInt;
             m_type |= SpecInt52AsDouble;
         }
-        RELEASE_ASSERT(!(m_type & ~SpecDouble));
+        RELEASE_ASSERT(!(m_type & ~SpecFullDouble));
     } else if (representation == NodeResultInt52) {
         if (m_type & SpecInt52AsDouble) {
             m_type &= ~SpecInt52AsDouble;

Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (167415 => 167416)


--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2014-04-17 04:57:29 UTC (rev 167416)
@@ -268,7 +268,7 @@
                 // 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, SpecDouble, *node);
+                    m_indexInBlock, SpecBytecodeDouble, *node);
                 newDivision->setResult(NodeResultDouble);
                 
                 node->setOp(DoubleAsInt32);
@@ -1881,7 +1881,7 @@
                     Edge(edge.node(), Int52RepUse));
             } else {
                 result = m_insertionSet.insertNode(
-                    m_indexInBlock, SpecDouble, DoubleRep, node->origin,
+                    m_indexInBlock, SpecBytecodeDouble, DoubleRep, node->origin,
                     Edge(edge.node(), NumberUse));
             }
 
@@ -1924,7 +1924,7 @@
             Node* result;
             if (edge->hasDoubleResult()) {
                 result = m_insertionSet.insertNode(
-                    m_indexInBlock, SpecDouble, ValueRep, node->origin,
+                    m_indexInBlock, SpecBytecodeDouble, ValueRep, node->origin,
                     Edge(edge.node(), DoubleRepUse));
             } else {
                 result = m_insertionSet.insertNode(

Modified: trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp (167415 => 167416)


--- trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp	2014-04-17 04:57:29 UTC (rev 167416)
@@ -287,7 +287,7 @@
             // before and after setting it.
             source = forNode(node->child1());
             if (node->variableAccessData()->flushFormat() == FlushedDouble)
-                RELEASE_ASSERT(!(source.m_type & ~SpecDouble));
+                RELEASE_ASSERT(!(source.m_type & ~SpecFullDouble));
             break;
         
         default:

Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (167415 => 167416)


--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2014-04-17 04:57:29 UTC (rev 167416)
@@ -373,7 +373,7 @@
             
             if (node->child1()->shouldSpeculateFloat32Array()
                 || node->child1()->shouldSpeculateFloat64Array())
-                changed |= mergePrediction(SpecDouble);
+                changed |= mergePrediction(SpecFullDouble);
             else if (node->child1()->shouldSpeculateUint32Array()) {
                 if (isInt32Speculation(node->getHeapPrediction()))
                     changed |= mergePrediction(SpecInt32);

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (167415 => 167416)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2014-04-17 04:57:29 UTC (rev 167416)
@@ -2163,6 +2163,13 @@
         
         FPRReg valueFPR = value.fpr();
         JSValueRegs resultRegs = result.regs();
+        
+        // It's very tempting to in-place filter the value to indicate that it's not impure NaN
+        // anymore. Unfortunately, this would be unsound. If it's a GetLocal or if the value was
+        // subject to a prior SetLocal, filtering the value would imply that the corresponding
+        // local was purified.
+        if (needsTypeCheck(node->child1(), ~SpecDoubleImpureNaN))
+            m_jit.purifyNaN(valueFPR);
 
 #if CPU(X86)
         // boxDouble() on X86 clobbers the source, so we need to copy.
@@ -2507,8 +2514,6 @@
         RELEASE_ASSERT_NOT_REACHED();
     }
     
-    m_jit.purifyNaN(resultReg);
-    
     doubleResult(resultReg, node);
 }
 

Modified: trunk/Source/_javascript_Core/dfg/DFGUseKind.h (167415 => 167416)


--- trunk/Source/_javascript_Core/dfg/DFGUseKind.h	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/dfg/DFGUseKind.h	2014-04-17 04:57:29 UTC (rev 167416)
@@ -73,7 +73,7 @@
     case NumberUse:
         return SpecBytecodeNumber;
     case DoubleRepUse:
-        return SpecDouble;
+        return SpecFullDouble;
     case DoubleRepRealUse:
         return SpecDoubleReal;
     case BooleanUse:

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (167415 => 167416)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2014-04-17 04:57:29 UTC (rev 167416)
@@ -743,7 +743,14 @@
     {
         switch (m_node->child1().useKind()) {
         case DoubleRepUse: {
-            setJSValue(boxDouble(lowDouble(m_node->child1())));
+            LValue value = lowDouble(m_node->child1());
+            
+            if (m_interpreter.needsTypeCheck(m_node->child1(), ~SpecDoubleImpureNaN)) {
+                value = m_out.select(
+                    m_out.doubleEqual(value, value), value, m_out.constDouble(PNaN));
+            }
+            
+            setJSValue(boxDouble(value));
             return;
         }
             
@@ -2127,8 +2134,6 @@
                     RELEASE_ASSERT_NOT_REACHED();
                 }
                 
-                result = m_out.select(
-                    m_out.doubleEqual(result, result), result, m_out.constDouble(PNaN));
                 setDouble(result);
                 return;
             }

Modified: trunk/Source/_javascript_Core/runtime/PureNaN.h (167415 => 167416)


--- trunk/Source/_javascript_Core/runtime/PureNaN.h	2014-04-17 03:54:11 UTC (rev 167415)
+++ trunk/Source/_javascript_Core/runtime/PureNaN.h	2014-04-17 04:57:29 UTC (rev 167416)
@@ -59,7 +59,7 @@
 // having to call purifyNaN() in surprising places.
 //
 // For naming purposes, we say that a NaN is "pure" if it is safe to tag, in the sense
-// that doing so would result in a tagged value that would base the "are you a double"
+// that doing so would result in a tagged value that would pass the "are you a double"
 // test. We say that a NaN is "impure" if attempting to tag it would result in a value
 // that would look like something other than a double.
 

Added: trunk/Source/_javascript_Core/tests/stress/float32-array-nan-inlined.js (0 => 167416)


--- trunk/Source/_javascript_Core/tests/stress/float32-array-nan-inlined.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/float32-array-nan-inlined.js	2014-04-17 04:57:29 UTC (rev 167416)
@@ -0,0 +1,20 @@
+function foo(o) {
+    return o[0];
+}
+
+function test(a, x) {
+    var intArray = new Int32Array(1);
+    intArray[0] = a;
+    var floatArray = new Float32Array(intArray.buffer);
+    var element = foo(floatArray);
+    var result = element + 1;
+    if (("" + result) != ("" + x))
+        throw "Error: bad result for " + a + ": " + result + ", but expected: " + x + "; loaded " + element + " from the array";
+}
+
+noInline(test);
+
+for (var i = 0; i < 100000; ++i)
+    test(0, 1);
+
+test(0xFFFF0000, 0/0);

Added: trunk/Source/_javascript_Core/tests/stress/float32-array-nan.js (0 => 167416)


--- trunk/Source/_javascript_Core/tests/stress/float32-array-nan.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/float32-array-nan.js	2014-04-17 04:57:29 UTC (rev 167416)
@@ -0,0 +1,20 @@
+function foo(o) {
+    return o[0];
+}
+
+noInline(foo);
+
+function test(a, x) {
+    var intArray = new Int32Array(1);
+    intArray[0] = a;
+    var floatArray = new Float32Array(intArray.buffer);
+    var element = foo(floatArray);
+    var result = element + 1;
+    if (("" + result) != ("" + x))
+        throw "Error: bad result for " + a + ": " + result + ", but expected: " + x + "; loaded " + element + " from the array";
+}
+
+for (var i = 0; i < 100000; ++i)
+    test(0, 1);
+
+test(0xFFFF0000, 0/0);

Added: trunk/Source/_javascript_Core/tests/stress/float64-array-nan-inlined.js (0 => 167416)


--- trunk/Source/_javascript_Core/tests/stress/float64-array-nan-inlined.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/float64-array-nan-inlined.js	2014-04-17 04:57:29 UTC (rev 167416)
@@ -0,0 +1,34 @@
+function foo(o) {
+    return o[0];
+}
+
+function isBigEndian() {
+    var word = new Int16Array(1);
+    word[0] = 1;
+    var bytes = new Int8Array(word.buffer);
+    return !bytes[0];
+}
+
+function test(a, b, x) {
+    var intArray = new Int32Array(2);
+    intArray[0] = a;
+    intArray[1] = b;
+    var floatArray = new Float64Array(intArray.buffer);
+    var element = foo(floatArray);
+    var result = element + 1;
+    if (("" + result) != ("" + x))
+        throw "Error: bad result for " + a + ", " + b + ": " + result + ", but expected: " + x + "; loaded " + element + " from the array";
+}
+
+noInline(test);
+
+for (var i = 0; i < 100000; ++i)
+    test(0, 0, 1);
+
+if (isBigEndian()) {
+    test(0xFFFF0000, 0, 0/0);
+    test(0, 0xFFFF0000, 1);
+} else {
+    test(0xFFFF0000, 0, 1);
+    test(0, 0xFFFF0000, 0/0);
+}

Added: trunk/Source/_javascript_Core/tests/stress/float64-array-nan.js (0 => 167416)


--- trunk/Source/_javascript_Core/tests/stress/float64-array-nan.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/float64-array-nan.js	2014-04-17 04:57:29 UTC (rev 167416)
@@ -0,0 +1,34 @@
+function foo(o) {
+    return o[0];
+}
+
+noInline(foo);
+
+function isBigEndian() {
+    var word = new Int16Array(1);
+    word[0] = 1;
+    var bytes = new Int8Array(word.buffer);
+    return !bytes[0];
+}
+
+function test(a, b, x) {
+    var intArray = new Int32Array(2);
+    intArray[0] = a;
+    intArray[1] = b;
+    var floatArray = new Float64Array(intArray.buffer);
+    var element = foo(floatArray);
+    var result = element + 1;
+    if (("" + result) != ("" + x))
+        throw "Error: bad result for " + a + ", " + b + ": " + result + ", but expected: " + x + "; loaded " + element + " from the array";
+}
+
+for (var i = 0; i < 100000; ++i)
+    test(0, 0, 1);
+
+if (isBigEndian()) {
+    test(0xFFFF0000, 0, 0/0);
+    test(0, 0xFFFF0000, 1);
+} else {
+    test(0xFFFF0000, 0, 1);
+    test(0, 0xFFFF0000, 0/0);
+}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to