Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (91207 => 91208)
--- trunk/Source/_javascript_Core/ChangeLog 2011-07-18 20:56:42 UTC (rev 91207)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-07-18 21:04:51 UTC (rev 91208)
@@ -1,3 +1,31 @@
+2011-07-18 Filip Pizlo <[email protected]>
+
+ DFG JIT does not optimize equal-null comparisons and branches.
+ https://bugs.webkit.org/show_bug.cgi?id=64659
+
+ Reviewed by Gavin Barraclough.
+
+ Added a peephole-aware compare-to-null implementation to JITCodeGenerator,
+ which is used by both the speculative and non-speculative JIT. Through
+ the use of the new isNullConstant helper, the two JITs invoke the
+ nonSpecualtiveCompareNull() helper instead of their regular comparison
+ helpers when compiling CompareEq. Through the use of the new isKnownCell
+ helper, the compare-null code will skip the is-a-cell check if the
+ speculative JIT had been speculating cell.
+
+ * dfg/DFGJITCodeGenerator.cpp:
+ (JSC::DFG::JITCodeGenerator::isKnownCell):
+ (JSC::DFG::JITCodeGenerator::nonSpeculativeNonPeepholeCompareNull):
+ (JSC::DFG::JITCodeGenerator::nonSpeculativePeepholeBranchNull):
+ (JSC::DFG::JITCodeGenerator::nonSpeculativeCompareNull):
+ * dfg/DFGJITCodeGenerator.h:
+ (JSC::DFG::JITCodeGenerator::isNullConstant):
+ * dfg/DFGNonSpeculativeJIT.cpp:
+ (JSC::DFG::NonSpeculativeJIT::compile):
+ * dfg/DFGOperations.cpp:
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+
2011-07-18 James Robinson <[email protected]>
Timer scheduling should be based off the monotonic clock
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp (91207 => 91208)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-07-18 20:56:42 UTC (rev 91207)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-07-18 21:04:51 UTC (rev 91208)
@@ -365,6 +365,21 @@
return false;
}
+bool JITCodeGenerator::isKnownCell(NodeIndex nodeIndex)
+{
+ GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
+
+ DataFormat registerFormat = info.registerFormat();
+ if (registerFormat != DataFormatNone)
+ return (registerFormat | DataFormatJS) == DataFormatJSCell;
+
+ DataFormat spillFormat = info.spillFormat();
+ if (spillFormat != DataFormatNone)
+ return (spillFormat | DataFormatJS) == DataFormatJSCell;
+
+ return false;
+}
+
bool JITCodeGenerator::isKnownNotInteger(NodeIndex nodeIndex)
{
Node& node = m_jit.graph()[nodeIndex];
@@ -519,6 +534,99 @@
m_jit.addMethodGet(slowCall, structToCompare, protoObj, protoStructToCompare, putFunction);
}
+void JITCodeGenerator::nonSpeculativeNonPeepholeCompareNull(NodeIndex operand, bool invert)
+{
+ JSValueOperand arg(this, operand);
+ GPRReg argGPR = arg.gpr();
+
+ GPRTemporary result(this, arg);
+ GPRReg resultGPR = result.gpr();
+
+ JITCompiler::Jump notCell;
+
+ if (!isKnownCell(operand))
+ notCell = m_jit.branchTestPtr(MacroAssembler::NonZero, argGPR, GPRInfo::tagMaskRegister);
+
+ m_jit.loadPtr(JITCompiler::Address(argGPR, JSCell::structureOffset()), resultGPR);
+ m_jit.test8(invert ? JITCompiler::Zero : JITCompiler::NonZero, JITCompiler::Address(resultGPR, Structure::typeInfoFlagsOffset()), JITCompiler::TrustedImm32(MasqueradesAsUndefined), resultGPR);
+
+ if (!isKnownCell(operand)) {
+ JITCompiler::Jump done = m_jit.jump();
+
+ notCell.link(&m_jit);
+
+ m_jit.move(argGPR, resultGPR);
+ m_jit.andPtr(JITCompiler::TrustedImm32(~TagBitUndefined), resultGPR);
+ m_jit.comparePtr(invert ? JITCompiler::NotEqual : JITCompiler::Equal, resultGPR, JITCompiler::TrustedImm32(ValueNull), resultGPR);
+
+ done.link(&m_jit);
+ }
+
+ m_jit.or32(TrustedImm32(ValueFalse), resultGPR);
+ jsValueResult(resultGPR, m_compileIndex);
+}
+
+void JITCodeGenerator::nonSpeculativePeepholeBranchNull(NodeIndex operand, NodeIndex branchNodeIndex, bool invert)
+{
+ Node& branchNode = m_jit.graph()[branchNodeIndex];
+ BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.takenBytecodeOffset());
+ BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.notTakenBytecodeOffset());
+
+ if (taken == (m_block + 1)) {
+ invert = !invert;
+ BlockIndex tmp = taken;
+ taken = notTaken;
+ notTaken = tmp;
+ }
+
+ JSValueOperand arg(this, operand);
+ GPRReg argGPR = arg.gpr();
+
+ GPRTemporary result(this, arg);
+ GPRReg resultGPR = result.gpr();
+
+ JITCompiler::Jump notCell;
+
+ if (!isKnownCell(operand))
+ notCell = m_jit.branchTestPtr(MacroAssembler::NonZero, argGPR, GPRInfo::tagMaskRegister);
+
+ m_jit.loadPtr(JITCompiler::Address(argGPR, JSCell::structureOffset()), resultGPR);
+ addBranch(m_jit.branchTest8(invert ? JITCompiler::Zero : JITCompiler::NonZero, JITCompiler::Address(resultGPR, Structure::typeInfoFlagsOffset()), JITCompiler::TrustedImm32(MasqueradesAsUndefined)), taken);
+
+ if (!isKnownCell(operand)) {
+ addBranch(m_jit.jump(), notTaken);
+
+ notCell.link(&m_jit);
+
+ m_jit.move(argGPR, resultGPR);
+ m_jit.andPtr(JITCompiler::TrustedImm32(~TagBitUndefined), resultGPR);
+ addBranch(m_jit.branchPtr(invert ? JITCompiler::NotEqual : JITCompiler::Equal, resultGPR, JITCompiler::TrustedImmPtr(reinterpret_cast<void*>(ValueNull))), taken);
+ }
+
+ if (notTaken != (m_block + 1))
+ addBranch(m_jit.jump(), notTaken);
+}
+
+bool JITCodeGenerator::nonSpeculativeCompareNull(Node& node, NodeIndex operand, bool invert)
+{
+ NodeIndex branchNodeIndex = detectPeepHoleBranch();
+ if (branchNodeIndex != NoNode) {
+ ASSERT(node.adjustedRefCount() == 1);
+
+ nonSpeculativePeepholeBranchNull(operand, branchNodeIndex, invert);
+
+ use(node.child1());
+ use(node.child2());
+ m_compileIndex = branchNodeIndex;
+
+ return true;
+ }
+
+ nonSpeculativeNonPeepholeCompareNull(operand, invert);
+
+ return false;
+}
+
void JITCodeGenerator::nonSpeculativePeepholeBranch(Node& node, NodeIndex branchNodeIndex, MacroAssembler::RelationalCondition cond, Z_DFGOperation_EJJ helperFunction)
{
Node& branchNode = m_jit.graph()[branchNodeIndex];
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h (91207 => 91208)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-07-18 20:56:42 UTC (rev 91207)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-07-18 21:04:51 UTC (rev 91208)
@@ -414,6 +414,7 @@
bool isKnownInteger(NodeIndex);
bool isKnownNumeric(NodeIndex);
+ bool isKnownCell(NodeIndex);
bool isKnownNotInteger(NodeIndex);
@@ -425,6 +426,12 @@
int32_t valueOfInt32Constant(NodeIndex nodeIndex) { return m_jit.valueOfInt32Constant(nodeIndex); }
double valueOfDoubleConstant(NodeIndex nodeIndex) { return m_jit.valueOfDoubleConstant(nodeIndex); }
JSValue valueOfJSConstant(NodeIndex nodeIndex) { return m_jit.valueOfJSConstant(nodeIndex); }
+ bool isNullConstant(NodeIndex nodeIndex)
+ {
+ if (!isConstant(nodeIndex))
+ return false;
+ return valueOfJSConstant(nodeIndex).isNull();
+ }
Identifier* identifier(unsigned index)
{
@@ -556,6 +563,10 @@
void cachedPutById(GPRReg baseGPR, GPRReg valueGPR, GPRReg scratchGPR, unsigned identifierNumber, PutKind, JITCompiler::Jump slowPathTarget = JITCompiler::Jump());
void cachedGetMethod(GPRReg baseGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget = JITCompiler::Jump());
+ void nonSpeculativeNonPeepholeCompareNull(NodeIndex operand, bool invert = false);
+ void nonSpeculativePeepholeBranchNull(NodeIndex operand, NodeIndex branchNodeIndex, bool invert = false);
+ bool nonSpeculativeCompareNull(Node&, NodeIndex operand, bool invert = false);
+
void nonSpeculativePeepholeBranch(Node&, NodeIndex branchNodeIndex, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ helperFunction);
void nonSpeculativeNonPeepholeCompare(Node&, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ helperFunction);
bool nonSpeculativeCompare(Node&, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ helperFunction);
Modified: trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp (91207 => 91208)
--- trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-07-18 20:56:42 UTC (rev 91207)
+++ trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-07-18 21:04:51 UTC (rev 91208)
@@ -673,6 +673,16 @@
break;
case CompareEq:
+ if (isNullConstant(node.child1())) {
+ if (nonSpeculativeCompareNull(node, node.child2()))
+ return;
+ break;
+ }
+ if (isNullConstant(node.child2())) {
+ if (nonSpeculativeCompareNull(node, node.child1()))
+ return;
+ break;
+ }
if (nonSpeculativeCompare(node, MacroAssembler::Equal, operationCompareEq))
return;
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (91207 => 91208)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-07-18 20:56:42 UTC (rev 91207)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-07-18 21:04:51 UTC (rev 91208)
@@ -767,6 +767,16 @@
break;
case CompareEq:
+ if (isNullConstant(node.child1())) {
+ if (nonSpeculativeCompareNull(node, node.child2()))
+ return;
+ break;
+ }
+ if (isNullConstant(node.child2())) {
+ if (nonSpeculativeCompareNull(node, node.child1()))
+ return;
+ break;
+ }
if (compare(node, JITCompiler::Equal, operationCompareEq))
return;
break;
