Title: [96894] trunk/Source/_javascript_Core
Revision
96894
Author
fpi...@apple.com
Date
2011-10-06 19:48:47 -0700 (Thu, 06 Oct 2011)

Log Message

DFG should not always speculate that ConvertThis is operating on an object
https://bugs.webkit.org/show_bug.cgi?id=69570

Reviewed by Oliver Hunt.
        
Mostly neutral, but with a slight regression in Kraken since it increases
coverage in DFG and thus reveals some performance pathologies (which I
prefer to think of as performance opportunities, in a good way).

* bytecode/PredictedType.cpp:
(JSC::predictionToString):
* bytecode/PredictedType.h:
(JSC::isOtherPrediction):
(JSC::mergePredictions):
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::propagateNodePredictions):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (96893 => 96894)


--- trunk/Source/_javascript_Core/ChangeLog	2011-10-07 02:44:52 UTC (rev 96893)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-10-07 02:48:47 UTC (rev 96894)
@@ -1,3 +1,26 @@
+2011-10-06  Filip Pizlo  <fpi...@apple.com>
+
+        DFG should not always speculate that ConvertThis is operating on an object
+        https://bugs.webkit.org/show_bug.cgi?id=69570
+
+        Reviewed by Oliver Hunt.
+        
+        Mostly neutral, but with a slight regression in Kraken since it increases
+        coverage in DFG and thus reveals some performance pathologies (which I
+        prefer to think of as performance opportunities, in a good way).
+
+        * bytecode/PredictedType.cpp:
+        (JSC::predictionToString):
+        * bytecode/PredictedType.h:
+        (JSC::isOtherPrediction):
+        (JSC::mergePredictions):
+        * dfg/DFGPropagator.cpp:
+        (JSC::DFG::Propagator::propagateNodePredictions):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+
 2011-10-06  Mark Hahnenberg  <mhahnenb...@apple.com>
 
         Windows build fix

Modified: trunk/Source/_javascript_Core/bytecode/PredictedType.cpp (96893 => 96894)


--- trunk/Source/_javascript_Core/bytecode/PredictedType.cpp	2011-10-07 02:44:52 UTC (rev 96893)
+++ trunk/Source/_javascript_Core/bytecode/PredictedType.cpp	2011-10-07 02:48:47 UTC (rev 96894)
@@ -44,11 +44,6 @@
     static char description[size];
     BoundsCheckedPointer<char> ptr(description, size);
     
-    if (value & PredictObjectUnknown) {
-        ASSERT(!(value & (PredictObjectMask & ~PredictObjectUnknown)));
-        ptr.strcat("Object");
-    }
-
     if (value & PredictCellOther)
         ptr.strcat("Othercell");
     

Modified: trunk/Source/_javascript_Core/bytecode/PredictedType.h (96893 => 96894)


--- trunk/Source/_javascript_Core/bytecode/PredictedType.h	2011-10-07 02:44:52 UTC (rev 96893)
+++ trunk/Source/_javascript_Core/bytecode/PredictedType.h	2011-10-07 02:48:47 UTC (rev 96894)
@@ -40,7 +40,6 @@
 static const PredictedType PredictFinalObject   = 0x0001; // It's definitely a JSFinalObject.
 static const PredictedType PredictArray         = 0x0002; // It's definitely a JSArray.
 static const PredictedType PredictObjectOther   = 0x0010; // It's definitely an object but not JSFinalObject or JSArray.
-static const PredictedType PredictObjectUnknown = 0x0020; // It's definitely an object, but we didn't record enough informatin to know more.
 static const PredictedType PredictObjectMask    = 0x003f; // Bitmask used for testing for any kind of object prediction.
 static const PredictedType PredictString        = 0x0040; // It's definitely a JSString.
 static const PredictedType PredictCellOther     = 0x0080; // It's definitely a JSCell but not a subclass of JSObject and definitely not a JSString.
@@ -107,21 +106,17 @@
     return value == PredictBoolean;
 }
 
+inline bool isOtherPrediction(PredictedType value)
+{
+    return value == PredictOther;
+}
+
 #ifndef NDEBUG
 const char* predictionToString(PredictedType value);
 #endif
 
 inline PredictedType mergePredictions(PredictedType left, PredictedType right)
 {
-    if (left & PredictObjectUnknown) {
-        ASSERT(!(left & (PredictObjectMask & ~PredictObjectUnknown)));
-        if (right & PredictObjectMask)
-            return (left & ~PredictObjectUnknown) | right;
-    } else if (right & PredictObjectUnknown) {
-        ASSERT(!(right & (PredictObjectMask & ~PredictObjectUnknown)));
-        if (left & PredictObjectMask)
-            return (right & ~PredictObjectUnknown) | left;
-    }
     return left | right;
 }
 

Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp (96893 => 96894)


--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp	2011-10-07 02:44:52 UTC (rev 96893)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp	2011-10-07 02:48:47 UTC (rev 96894)
@@ -375,7 +375,7 @@
         return;
     }
 
-    GenerationInfo& childInfo = m_generationInfo[at(node.child1())].virtualRegister()];
+    GenerationInfo& childInfo = m_generationInfo[at(node.child1()).virtualRegister()];
     if (isJSDouble(childInfo.registerFormat())) {
         DoubleOperand op1(this, node.child1());
         GPRTemporary result(this);

Modified: trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp (96893 => 96894)


--- trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp	2011-10-07 02:44:52 UTC (rev 96893)
+++ trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp	2011-10-07 02:48:47 UTC (rev 96894)
@@ -478,7 +478,7 @@
             if (prediction) {
                 if (prediction & ~PredictObjectMask) {
                     prediction &= PredictObjectMask;
-                    prediction = mergePredictions(prediction, PredictObjectUnknown);
+                    prediction = mergePredictions(prediction, PredictObjectOther);
                 }
                 changed |= mergePrediction(prediction);
             }

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (96893 => 96894)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2011-10-07 02:44:52 UTC (rev 96893)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2011-10-07 02:48:47 UTC (rev 96894)
@@ -1710,11 +1710,42 @@
     }
         
     case ConvertThis: {
-        SpeculateCellOperand thisValue(this, node.child1());
-
-        speculationCheck(m_jit.branchPtr(JITCompiler::Equal, JITCompiler::Address(thisValue.gpr()), JITCompiler::TrustedImmPtr(m_jit.globalData()->jsStringVPtr)));
-
-        cellResult(thisValue.gpr(), m_compileIndex);
+        if (isOtherPrediction(node.prediction())) {
+            JSValueOperand thisValue(this, node.child1());
+            GPRTemporary scratch(this, thisValue);
+            
+            GPRReg thisValueTagGPR = thisValue.tagGPR();
+            GPRReg scratchGPR = scratch.gpr();
+            
+            m_jit.move(thisValueTagGPR, scratchGPR);
+            m_jit.and32(MacroAssembler::TrustedImm32(JSValue::UndefinedTag), scratchGPR);
+            speculationCheck(m_jit.branch32(MacroAssembler::NotEqual, scratchGPR, TrustedImm32(JSValue::UndefinedTag)));
+            
+            m_jit.move(MacroAssembler::TrustedImmPtr(m_jit.codeBlock()->globalObject()), scratchGPR);
+            cellResult(scratchGPR, m_compileIndex);
+            break;
+        }
+        
+        if (isObjectPrediction(node.prediction())) {
+            SpeculateCellOperand thisValue(this, node.child1());
+            
+            speculationCheck(m_jit.branchPtr(JITCompiler::Equal, JITCompiler::Address(thisValue.gpr()), JITCompiler::TrustedImmPtr(m_jit.globalData()->jsStringVPtr)));
+            
+            cellResult(thisValue.gpr(), m_compileIndex);
+            break;
+        }
+        
+        JSValueOperand thisValue(this, node.child1());
+        GPRReg thisValueTagGPR = thisValue.tagGPR();
+        GPRReg thisValuePayloadGPR = thisValue.payloadGPR();
+        
+        flushRegisters();
+        
+        GPRResult2 resultTag(this);
+        GPRResult resultPayload(this);
+        callOperation(operationConvertThis, resultTag.gpr(), resultPayload.gpr(), thisValueTagGPR, thisValuePayloadGPR);
+        
+        cellResult(resultPayload.gpr(), m_compileIndex);
         break;
     }
 

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (96893 => 96894)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2011-10-07 02:44:52 UTC (rev 96893)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2011-10-07 02:48:47 UTC (rev 96894)
@@ -1786,11 +1786,39 @@
     }
         
     case ConvertThis: {
-        SpeculateCellOperand thisValue(this, node.child1());
-
-        speculationCheck(m_jit.branchPtr(JITCompiler::Equal, JITCompiler::Address(thisValue.gpr()), JITCompiler::TrustedImmPtr(m_jit.globalData()->jsStringVPtr)));
-
-        cellResult(thisValue.gpr(), m_compileIndex);
+        if (isOtherPrediction(node.prediction())) {
+            JSValueOperand thisValue(this, node.child1());
+            GPRTemporary scratch(this, thisValue);
+            GPRReg thisValueGPR = thisValue.gpr();
+            GPRReg scratchGPR = scratch.gpr();
+            
+            m_jit.move(thisValueGPR, scratchGPR);
+            m_jit.andPtr(MacroAssembler::TrustedImm32(~TagBitUndefined), scratchGPR);
+            speculationCheck(m_jit.branchPtr(MacroAssembler::NotEqual, scratchGPR, MacroAssembler::TrustedImmPtr(reinterpret_cast<void*>(ValueNull))));
+            
+            m_jit.move(MacroAssembler::TrustedImmPtr(m_jit.codeBlock()->globalObject()), scratchGPR);
+            cellResult(scratchGPR, m_compileIndex);
+            break;
+        }
+        
+        if (isObjectPrediction(node.prediction())) {
+            SpeculateCellOperand thisValue(this, node.child1());
+            
+            speculationCheck(m_jit.branchPtr(JITCompiler::Equal, JITCompiler::Address(thisValue.gpr()), JITCompiler::TrustedImmPtr(m_jit.globalData()->jsStringVPtr)));
+            
+            cellResult(thisValue.gpr(), m_compileIndex);
+            break;
+        }
+        
+        JSValueOperand thisValue(this, node.child1());
+        GPRReg thisValueGPR = thisValue.gpr();
+        
+        flushRegisters();
+        
+        GPRResult result(this);
+        callOperation(operationConvertThis, result.gpr(), thisValueGPR);
+        
+        cellResult(result.gpr(), m_compileIndex);
         break;
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to