[webkit-changes] [93375] trunk/Source/JavaScriptCore

Thu, 18 Aug 2011 17:26:50 -0700

Title: [93375] trunk/Source/_javascript_Core
Revision
93375
Author
[email protected]
Date
2011-08-18 17:18:49 -0700 (Thu, 18 Aug 2011)

Log Message

DFG non-speculative JIT does not inline the double case of ValueAdd
https://bugs.webkit.org/show_bug.cgi?id=66025

Reviewed by Gavin Barraclough.
        
This is a 1.3% win on Kraken overall, with >=8% speed-ups on a few
benchmarks (imaging-darkroom, stanford-crypto-pbkdf2,
stanford-crypto-sha256-iterative).  It looks like it might have
a speed-up in SunSpider (though not statistically significant or
particularly reproducible) and a slight slow-down in V8 (0.14%,
not statistically significant).  It does slow down v8-crypto by
1.5%.

* dfg/DFGJITCodeGenerator.cpp:
(JSC::DFG::JITCodeGenerator::isKnownInteger):
(JSC::DFG::JITCodeGenerator::isKnownNumeric):
* dfg/DFGNonSpeculativeJIT.cpp:
(JSC::DFG::NonSpeculativeJIT::knownConstantArithOp):
(JSC::DFG::NonSpeculativeJIT::basicArithOp):
* dfg/DFGOperations.cpp:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (93374 => 93375)


--- trunk/Source/_javascript_Core/ChangeLog	2011-08-19 00:00:10 UTC (rev 93374)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-08-19 00:18:49 UTC (rev 93375)
@@ -1,3 +1,26 @@
+2011-08-10  Filip Pizlo  <[email protected]>
+
+        DFG non-speculative JIT does not inline the double case of ValueAdd
+        https://bugs.webkit.org/show_bug.cgi?id=66025
+
+        Reviewed by Gavin Barraclough.
+        
+        This is a 1.3% win on Kraken overall, with >=8% speed-ups on a few
+        benchmarks (imaging-darkroom, stanford-crypto-pbkdf2,
+        stanford-crypto-sha256-iterative).  It looks like it might have
+        a speed-up in SunSpider (though not statistically significant or
+        particularly reproducible) and a slight slow-down in V8 (0.14%,
+        not statistically significant).  It does slow down v8-crypto by
+        1.5%.
+
+        * dfg/DFGJITCodeGenerator.cpp:
+        (JSC::DFG::JITCodeGenerator::isKnownInteger):
+        (JSC::DFG::JITCodeGenerator::isKnownNumeric):
+        * dfg/DFGNonSpeculativeJIT.cpp:
+        (JSC::DFG::NonSpeculativeJIT::knownConstantArithOp):
+        (JSC::DFG::NonSpeculativeJIT::basicArithOp):
+        * dfg/DFGOperations.cpp:
+
 2011-08-18  Filip Pizlo  <[email protected]>
 
         [jsfunfuzz] DFG speculative JIT does divide-by-zero checks incorrectly

Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp (93374 => 93375)


--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp	2011-08-19 00:00:10 UTC (rev 93374)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp	2011-08-19 00:18:49 UTC (rev 93375)
@@ -330,7 +330,12 @@
     if (isInt32Constant(nodeIndex))
         return true;
 
-    GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
+    Node& node = m_jit.graph()[nodeIndex];
+    
+    if (node.hasInt32Result())
+        return true;
+    
+    GenerationInfo& info = m_generationInfo[node.virtualRegister()];
 
     DataFormat registerFormat = info.registerFormat();
     if (registerFormat != DataFormatNone)
@@ -349,7 +354,12 @@
     if (isInt32Constant(nodeIndex) || isDoubleConstant(nodeIndex))
         return true;
 
-    GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
+    Node& node = m_jit.graph()[nodeIndex];
+    
+    if (node.hasNumericResult())
+        return true;
+    
+    GenerationInfo& info = m_generationInfo[node.virtualRegister()];
 
     DataFormat registerFormat = info.registerFormat();
     if (registerFormat != DataFormatNone)

Modified: trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp (93374 => 93375)


--- trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp	2011-08-19 00:00:10 UTC (rev 93374)
+++ trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp	2011-08-19 00:18:49 UTC (rev 93375)
@@ -169,12 +169,68 @@
     
     overflow.link(&m_jit);
     
+    JITCompiler::Jump notNumber;
+    
+    // first deal with overflow case
+    m_jit.convertInt32ToDouble(regArgGPR, tmp2FPR);
+    
+    // now deal with not-int case, if applicable
+    if (!isKnownInteger(regChild)) {
+        JITCompiler::Jump haveValue = m_jit.jump();
+        
+        notInt.link(&m_jit);
+        
+        if (!isKnownNumeric(regChild)) {
+            ASSERT(op == ValueAdd);
+            notNumber = m_jit.branchTestPtr(MacroAssembler::Zero, regArgGPR, GPRInfo::tagTypeNumberRegister);
+        }
+        
+        m_jit.move(regArgGPR, resultGPR);
+        m_jit.addPtr(GPRInfo::tagTypeNumberRegister, resultGPR);
+        m_jit.movePtrToDouble(resultGPR, tmp2FPR);
+        
+        haveValue.link(&m_jit);
+    }
+    
+    m_jit.move(MacroAssembler::ImmPtr(reinterpret_cast<void*>(reinterpretDoubleToIntptr(valueOfDoubleConstant(immChild)))), resultGPR);
+    m_jit.movePtrToDouble(resultGPR, tmp1FPR);
     switch (op) {
     case ValueAdd:
-        // overflow and not-int are the same
-        if (!isKnownInteger(regChild))
-            notInt.link(&m_jit);
+    case ArithAdd:
+        m_jit.addDouble(tmp1FPR, tmp2FPR);
+        break;
         
+    case ArithSub:
+        m_jit.subDouble(tmp1FPR, tmp2FPR);
+        break;
+            
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    
+    JITCompiler::Jump doneCaseConvertedToInt;
+    
+    if (op == ValueAdd) {
+        JITCompiler::JumpList failureCases;
+        m_jit.branchConvertDoubleToInt32(tmp2FPR, resultGPR, failureCases, tmp1FPR);
+        m_jit.orPtr(GPRInfo::tagTypeNumberRegister, resultGPR);
+        
+        doneCaseConvertedToInt = m_jit.jump();
+        
+        failureCases.link(&m_jit);
+    }
+    
+    m_jit.moveDoubleToPtr(tmp2FPR, resultGPR);
+    m_jit.subPtr(GPRInfo::tagTypeNumberRegister, resultGPR);
+        
+    if (!isKnownNumeric(regChild)) {
+        ASSERT(notNumber.isSet());
+        ASSERT(op == ValueAdd);
+            
+        JITCompiler::Jump doneCaseWasNumber = m_jit.jump();
+            
+        notNumber.link(&m_jit);
+            
         silentSpillAllRegisters(resultGPR);
         if (commute) {
             m_jit.move(regArgGPR, GPRInfo::argumentGPR2);
@@ -187,39 +243,13 @@
         appendCallWithExceptionCheck(operationValueAdd);
         m_jit.move(GPRInfo::returnValueGPR, resultGPR);
         silentFillAllRegisters(resultGPR);
-        break;
-
-    case ArithAdd:
-    case ArithSub:
-        // first deal with overflow case
-        m_jit.convertInt32ToDouble(regArgGPR, tmp2FPR);
-        
-        // now deal with not-int case, if applicable
-        if (!isKnownInteger(regChild)) {
-            JITCompiler::Jump haveValue = m_jit.jump();
             
-            notInt.link(&m_jit);
-            
-            m_jit.move(regArgGPR, resultGPR);
-            unboxDouble(resultGPR, tmp2FPR);
-            
-            haveValue.link(&m_jit);
-        }
-        
-        m_jit.move(MacroAssembler::ImmPtr(reinterpret_cast<void*>(reinterpretDoubleToIntptr(valueOfDoubleConstant(immChild)))), resultGPR);
-        m_jit.movePtrToDouble(resultGPR, tmp1FPR);
-        if (op == ArithAdd)
-            m_jit.addDouble(tmp1FPR, tmp2FPR);
-        else
-            m_jit.subDouble(tmp1FPR, tmp2FPR);
-        boxDouble(tmp2FPR, resultGPR);
-        break;
-        
-    default:
-        ASSERT_NOT_REACHED();
+        doneCaseWasNumber.link(&m_jit);
     }
     
     done.link(&m_jit);
+    if (doneCaseConvertedToInt.isSet())
+        doneCaseConvertedToInt.link(&m_jit);
         
     jsValueResult(resultGPR, m_compileIndex, UseChildrenCalledExplicitly);
 }
@@ -279,86 +309,125 @@
         
     JITCompiler::Jump done = m_jit.jump();
     
-    if (op == ValueAdd) {
-        if (child1NotInt.isSet())
-            child1NotInt.link(&m_jit);
-        if (child2NotInt.isSet())
-            child2NotInt.link(&m_jit);
-        overflow.link(&m_jit);
-        
-        silentSpillAllRegisters(resultGPR);
-        setupStubArguments(arg1GPR, arg2GPR);
-        m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
-        appendCallWithExceptionCheck(operationValueAdd);
-        m_jit.move(GPRInfo::returnValueGPR, resultGPR);
-        silentFillAllRegisters(resultGPR);
-    } else {
-        JITCompiler::JumpList haveFPRArguments;
+    JITCompiler::JumpList haveFPRArguments;
 
-        overflow.link(&m_jit);
+    overflow.link(&m_jit);
         
-        // both arguments are integers
-        m_jit.convertInt32ToDouble(arg1GPR, tmp1FPR);
-        m_jit.convertInt32ToDouble(arg2GPR, tmp2FPR);
+    // both arguments are integers
+    m_jit.convertInt32ToDouble(arg1GPR, tmp1FPR);
+    m_jit.convertInt32ToDouble(arg2GPR, tmp2FPR);
         
-        haveFPRArguments.append(m_jit.jump());
+    haveFPRArguments.append(m_jit.jump());
         
-        JITCompiler::Jump child2NotInt2;
+    JITCompiler::JumpList notNumbers;
         
-        if (!isKnownInteger(node.child1())) {
-            child1NotInt.link(&m_jit);
+    JITCompiler::Jump child2NotInt2;
+        
+    if (!isKnownInteger(node.child1())) {
+        child1NotInt.link(&m_jit);
             
-            m_jit.move(arg1GPR, resultGPR);
-            unboxDouble(resultGPR, tmp1FPR);
+        if (!isKnownNumeric(node.child1())) {
+            ASSERT(op == ValueAdd);
+            notNumbers.append(m_jit.branchTestPtr(MacroAssembler::Zero, arg1GPR, GPRInfo::tagTypeNumberRegister));
+        }
             
-            // child1 is converted to a double; child2 may either be an int or
-            // a boxed double
+        m_jit.move(arg1GPR, resultGPR);
+        unboxDouble(resultGPR, tmp1FPR);
             
-            if (!isKnownInteger(node.child2()))
+        // child1 is converted to a double; child2 may either be an int or
+        // a boxed double
+            
+        if (!isKnownInteger(node.child2())) {
+            if (isKnownNumeric(node.child2()))
                 child2NotInt2 = m_jit.branchPtr(MacroAssembler::Below, arg2GPR, GPRInfo::tagTypeNumberRegister);
+            else {
+                ASSERT(op == ValueAdd);
+                JITCompiler::Jump child2IsInt = m_jit.branchPtr(MacroAssembler::AboveOrEqual, arg2GPR, GPRInfo::tagTypeNumberRegister);
+                notNumbers.append(m_jit.branchTestPtr(MacroAssembler::Zero, arg2GPR, GPRInfo::tagTypeNumberRegister));
+                child2NotInt2 = m_jit.jump();
+                child2IsInt.link(&m_jit);
+            }
+        }
             
-            // child 2 is definitely an integer
-            m_jit.convertInt32ToDouble(arg2GPR, tmp2FPR);
+        // child 2 is definitely an integer
+        m_jit.convertInt32ToDouble(arg2GPR, tmp2FPR);
             
-            haveFPRArguments.append(m_jit.jump());
-        }
+        haveFPRArguments.append(m_jit.jump());
+    }
         
-        if (!isKnownInteger(node.child2())) {
-            child2NotInt.link(&m_jit);
-            // child1 is definitely an integer, and child 2 is definitely not
+    if (!isKnownInteger(node.child2())) {
+        child2NotInt.link(&m_jit);
             
-            m_jit.convertInt32ToDouble(arg1GPR, tmp1FPR);
+        if (!isKnownNumeric(node.child2())) {
+            ASSERT(op == ValueAdd);
+            notNumbers.append(m_jit.branchTestPtr(MacroAssembler::Zero, arg2GPR, GPRInfo::tagTypeNumberRegister));
+        }
             
-            if (child2NotInt2.isSet())
-                child2NotInt2.link(&m_jit);
+        // child1 is definitely an integer, and child 2 is definitely not
             
-            m_jit.move(arg2GPR, resultGPR);
-            unboxDouble(resultGPR, tmp2FPR);
-        }
+        m_jit.convertInt32ToDouble(arg1GPR, tmp1FPR);
+            
+        if (child2NotInt2.isSet())
+            child2NotInt2.link(&m_jit);
+            
+        m_jit.move(arg2GPR, resultGPR);
+        unboxDouble(resultGPR, tmp2FPR);
+    }
         
-        haveFPRArguments.link(&m_jit);
+    haveFPRArguments.link(&m_jit);
         
-        switch (op) {
-        case ArithAdd:
-            m_jit.addDouble(tmp2FPR, tmp1FPR);
-            break;
+    switch (op) {
+    case ValueAdd:
+    case ArithAdd:
+        m_jit.addDouble(tmp2FPR, tmp1FPR);
+        break;
             
-        case ArithSub:
-            m_jit.subDouble(tmp2FPR, tmp1FPR);
-            break;
+    case ArithSub:
+        m_jit.subDouble(tmp2FPR, tmp1FPR);
+        break;
             
-        case ArithMul:
-            m_jit.mulDouble(tmp2FPR, tmp1FPR);
-            break;
+    case ArithMul:
+        m_jit.mulDouble(tmp2FPR, tmp1FPR);
+        break;
             
-        default:
-            ASSERT_NOT_REACHED();
-        }
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    
+    JITCompiler::Jump doneCaseConvertedToInt;
+    
+    if (op == ValueAdd) {
+        JITCompiler::JumpList failureCases;
+        m_jit.branchConvertDoubleToInt32(tmp1FPR, resultGPR, failureCases, tmp2FPR);
+        m_jit.orPtr(GPRInfo::tagTypeNumberRegister, resultGPR);
         
-        boxDouble(tmp1FPR, resultGPR);
+        doneCaseConvertedToInt = m_jit.jump();
+        
+        failureCases.link(&m_jit);
     }
+        
+    boxDouble(tmp1FPR, resultGPR);
+        
+    if (!notNumbers.empty()) {
+        ASSERT(op == ValueAdd);
+            
+        JITCompiler::Jump doneCaseWasNumber = m_jit.jump();
+            
+        notNumbers.link(&m_jit);
+            
+        silentSpillAllRegisters(resultGPR);
+        setupStubArguments(arg1GPR, arg2GPR);
+        m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+        appendCallWithExceptionCheck(operationValueAdd);
+        m_jit.move(GPRInfo::returnValueGPR, resultGPR);
+        silentFillAllRegisters(resultGPR);
+
+        doneCaseWasNumber.link(&m_jit);
+    }
     
     done.link(&m_jit);
+    if (doneCaseConvertedToInt.isSet())
+        doneCaseConvertedToInt.link(&m_jit);
         
     jsValueResult(resultGPR, m_compileIndex, UseChildrenCalledExplicitly);
 }

Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (93374 => 93375)


--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp	2011-08-19 00:00:10 UTC (rev 93374)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp	2011-08-19 00:18:49 UTC (rev 93375)
@@ -123,19 +123,8 @@
 {
     JSValue op1 = JSValue::decode(encodedOp1);
     JSValue op2 = JSValue::decode(encodedOp2);
-
-    if (op1.isInt32() && op2.isInt32()) {
-        int64_t result64 = static_cast<int64_t>(op1.asInt32()) + static_cast<int64_t>(op2.asInt32());
-        int32_t result32 = static_cast<int32_t>(result64);
-        if (LIKELY(result32 == result64))
-            return JSValue::encode(jsNumber(result32));
-        return JSValue::encode(jsNumber((double)result64));
-    }
     
-    double number1;
-    double number2;
-    if (op1.getNumber(number1) && op2.getNumber(number2))
-        return JSValue::encode(jsNumber(number1 + number2));
+    ASSERT(!op1.isNumber() || !op2.isNumber());
 
     return JSValue::encode(jsAddSlowCase(exec, op1, op2));
 }

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to