Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (91098 => 91099)
--- trunk/Source/_javascript_Core/ChangeLog 2011-07-15 20:10:07 UTC (rev 91098)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-07-15 20:14:27 UTC (rev 91099)
@@ -1,3 +1,36 @@
+2011-07-15 Filip Pizlo <[email protected]>
+
+ DFG JIT is inconsistent about fusing branches and speculating
+ integer comparisons for branches.
+ https://bugs.webkit.org/show_bug.cgi?id=64573
+
+ Reviewed by Gavin Barraclough.
+
+ This patch moves some of NonSpeculativeJIT's functionality up into the
+ JITCodeGenerator superclass so that it can be used from both JITs. Now,
+ in cases where the speculative JIT doesn't want to speculate but still
+ wants to emit good code, it can reliably emit the same code sequence as
+ the non-speculative JIT. This patch also extends the non-speculative
+ JIT's compare optimizations to include compare/branch fusing, and
+ extends the speculative JIT's compare optimizations to cover StrictEqual.
+
+ * dfg/DFGJITCodeGenerator.cpp:
+ (JSC::DFG::JITCodeGenerator::isKnownInteger):
+ (JSC::DFG::JITCodeGenerator::isKnownNumeric):
+ (JSC::DFG::JITCodeGenerator::nonSpeculativePeepholeBranch):
+ (JSC::DFG::JITCodeGenerator::nonSpeculativeCompare):
+ * dfg/DFGJITCodeGenerator.h:
+ (JSC::DFG::JITCodeGenerator::detectPeepHoleBranch):
+ * dfg/DFGNonSpeculativeJIT.cpp:
+ (JSC::DFG::NonSpeculativeJIT::compile):
+ * dfg/DFGNonSpeculativeJIT.h:
+ * dfg/DFGOperations.cpp:
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compare):
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT.h:
+ * wtf/Platform.h:
+
2011-07-14 Gavin Barraclough <[email protected]>
https://bugs.webkit.org/show_bug.cgi?id=64250
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp (91098 => 91099)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-07-15 20:10:07 UTC (rev 91098)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-07-15 20:14:27 UTC (rev 91099)
@@ -325,6 +325,46 @@
}
}
+bool JITCodeGenerator::isKnownInteger(NodeIndex nodeIndex)
+{
+ if (isInt32Constant(nodeIndex))
+ return true;
+
+ GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
+
+ DataFormat registerFormat = info.registerFormat();
+ if (registerFormat != DataFormatNone)
+ return (registerFormat | DataFormatJS) == DataFormatJSInteger;
+
+ DataFormat spillFormat = info.spillFormat();
+ if (spillFormat != DataFormatNone)
+ return (spillFormat | DataFormatJS) == DataFormatJSInteger;
+
+ ASSERT(isConstant(nodeIndex));
+ return false;
+}
+
+bool JITCodeGenerator::isKnownNumeric(NodeIndex nodeIndex)
+{
+ if (isInt32Constant(nodeIndex) || isDoubleConstant(nodeIndex))
+ return true;
+
+ GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
+
+ DataFormat registerFormat = info.registerFormat();
+ if (registerFormat != DataFormatNone)
+ return (registerFormat | DataFormatJS) == DataFormatJSInteger
+ || (registerFormat | DataFormatJS) == DataFormatJSDouble;
+
+ DataFormat spillFormat = info.spillFormat();
+ if (spillFormat != DataFormatNone)
+ return (spillFormat | DataFormatJS) == DataFormatJSInteger
+ || (spillFormat | DataFormatJS) == DataFormatJSDouble;
+
+ ASSERT(isConstant(nodeIndex));
+ return false;
+}
+
JITCompiler::Call JITCodeGenerator::cachedGetById(GPRReg baseGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget, NodeType nodeType)
{
GPRReg scratchGPR;
@@ -468,6 +508,116 @@
m_jit.addMethodGet(slowCall, structToCompare, protoObj, protoStructToCompare, putFunction);
}
+void JITCodeGenerator::nonSpeculativePeepholeBranch(Node& node, NodeIndex branchNodeIndex, MacroAssembler::RelationalCondition cond, Z_DFGOperation_EJJ helperFunction)
+{
+ Node& branchNode = m_jit.graph()[branchNodeIndex];
+ BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.takenBytecodeOffset());
+ BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.notTakenBytecodeOffset());
+
+ JITCompiler::ResultCondition callResultCondition = JITCompiler::NonZero;
+
+ // The branch instruction will branch to the taken block.
+ // If taken is next, switch taken with notTaken & invert the branch condition so we can fall through.
+ if (taken == (m_block + 1)) {
+ cond = JITCompiler::invert(cond);
+ callResultCondition = JITCompiler::Zero;
+ BlockIndex tmp = taken;
+ taken = notTaken;
+ notTaken = tmp;
+ }
+
+ JSValueOperand arg1(this, node.child1());
+ JSValueOperand arg2(this, node.child2());
+ GPRReg arg1GPR = arg1.gpr();
+ GPRReg arg2GPR = arg2.gpr();
+
+ GPRTemporary result(this, arg2);
+ GPRReg resultGPR = result.gpr();
+
+ JITCompiler::JumpList slowPath;
+
+ if (!isKnownInteger(node.child1()))
+ slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg1GPR, GPRInfo::tagTypeNumberRegister));
+ if (!isKnownInteger(node.child2()))
+ slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg2GPR, GPRInfo::tagTypeNumberRegister));
+
+ addBranch(m_jit.branch32(cond, arg1GPR, arg2GPR), taken);
+
+ if (!isKnownInteger(node.child1()) || !isKnownInteger(node.child2())) {
+ addBranch(m_jit.jump(), notTaken);
+
+ slowPath.link(&m_jit);
+
+ silentSpillAllRegisters(resultGPR);
+ setupStubArguments(arg1GPR, arg2GPR);
+ m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+ appendCallWithExceptionCheck(helperFunction);
+ m_jit.move(GPRInfo::returnValueGPR, resultGPR);
+ silentFillAllRegisters(resultGPR);
+
+ addBranch(m_jit.branchTest8(callResultCondition, resultGPR), taken);
+ }
+
+ if (notTaken != (m_block + 1))
+ addBranch(m_jit.jump(), notTaken);
+}
+
+bool JITCodeGenerator::nonSpeculativeCompare(Node& node, MacroAssembler::RelationalCondition cond, Z_DFGOperation_EJJ helperFunction)
+{
+ NodeIndex branchNodeIndex = detectPeepHoleBranch();
+ if (branchNodeIndex != NoNode) {
+ ASSERT(node.adjustedRefCount() == 1);
+
+ nonSpeculativePeepholeBranch(node, branchNodeIndex, cond, helperFunction);
+
+ use(node.child1());
+ use(node.child2());
+ m_compileIndex = branchNodeIndex;
+
+ return true;
+ }
+
+ JSValueOperand arg1(this, node.child1());
+ JSValueOperand arg2(this, node.child2());
+ GPRReg arg1GPR = arg1.gpr();
+ GPRReg arg2GPR = arg2.gpr();
+
+ GPRTemporary result(this, arg2);
+ GPRReg resultGPR = result.gpr();
+
+ JITCompiler::JumpList slowPath;
+
+ if (!isKnownInteger(node.child1()))
+ slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg1GPR, GPRInfo::tagTypeNumberRegister));
+ if (!isKnownInteger(node.child2()))
+ slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg2GPR, GPRInfo::tagTypeNumberRegister));
+
+ m_jit.compare32(cond, arg1GPR, arg2GPR, resultGPR);
+
+ if (!isKnownInteger(node.child1()) || !isKnownInteger(node.child2())) {
+ JITCompiler::Jump haveResult = m_jit.jump();
+
+ slowPath.link(&m_jit);
+
+ silentSpillAllRegisters(resultGPR);
+ setupStubArguments(arg1GPR, arg2GPR);
+ m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+ appendCallWithExceptionCheck(helperFunction);
+ m_jit.move(GPRInfo::returnValueGPR, resultGPR);
+ silentFillAllRegisters(resultGPR);
+
+ m_jit.andPtr(TrustedImm32(1), resultGPR);
+
+ haveResult.link(&m_jit);
+ }
+
+ m_jit.or32(TrustedImm32(ValueFalse), resultGPR);
+
+ jsValueResult(resultGPR, m_compileIndex);
+
+ return false;
+}
+
void JITCodeGenerator::emitBranch(Node& node)
{
JSValueOperand value(this, node.child1());
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h (91098 => 91099)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-07-15 20:10:07 UTC (rev 91098)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-07-15 20:14:27 UTC (rev 91099)
@@ -412,6 +412,9 @@
info.spill((DataFormat)(spillFormat | DataFormatJS));
}
+ bool isKnownInteger(NodeIndex);
+ bool isKnownNumeric(NodeIndex);
+
// Checks/accessors for constant values.
bool isConstant(NodeIndex nodeIndex) { return m_jit.isConstant(nodeIndex); }
bool isJSConstant(NodeIndex nodeIndex) { return m_jit.isJSConstant(nodeIndex); }
@@ -531,10 +534,29 @@
}
}
+ // Returns the node index of the branch node if peephole is okay, NoNode otherwise.
+ NodeIndex detectPeepHoleBranch()
+ {
+ NodeIndex lastNodeIndex = m_jit.graph().m_blocks[m_block]->end - 1;
+
+ // Check that no intervening nodes will be generated.
+ for (NodeIndex index = m_compileIndex + 1; index < lastNodeIndex; ++index) {
+ if (m_jit.graph()[index].shouldGenerate())
+ return NoNode;
+ }
+
+ // Check if the lastNode is a branch on this node.
+ Node& lastNode = m_jit.graph()[lastNodeIndex];
+ return lastNode.op == Branch && lastNode.child1() == m_compileIndex ? lastNodeIndex : NoNode;
+ }
+
JITCompiler::Call cachedGetById(GPRReg baseGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget = JITCompiler::Jump(), NodeType = GetById);
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 nonSpeculativePeepholeBranch(Node&, NodeIndex branchNodeIndex, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ helperFunction);
+ bool nonSpeculativeCompare(Node&, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ helperFunction);
+
void emitBranch(Node&);
MacroAssembler::Address addressOfCallData(int idx)
Modified: trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp (91098 => 91099)
--- trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-07-15 20:10:07 UTC (rev 91098)
+++ trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-07-15 20:14:27 UTC (rev 91099)
@@ -120,46 +120,6 @@
truncatedToInteger.link(&m_jit);
}
-bool NonSpeculativeJIT::isKnownInteger(NodeIndex nodeIndex)
-{
- if (isInt32Constant(nodeIndex))
- return true;
-
- GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
-
- DataFormat registerFormat = info.registerFormat();
- if (registerFormat != DataFormatNone)
- return (registerFormat | DataFormatJS) == DataFormatJSInteger;
-
- DataFormat spillFormat = info.spillFormat();
- if (spillFormat != DataFormatNone)
- return (spillFormat | DataFormatJS) == DataFormatJSInteger;
-
- ASSERT(isConstant(nodeIndex));
- return false;
-}
-
-bool NonSpeculativeJIT::isKnownNumeric(NodeIndex nodeIndex)
-{
- if (isInt32Constant(nodeIndex) || isDoubleConstant(nodeIndex))
- return true;
-
- GenerationInfo& info = m_generationInfo[m_jit.graph()[nodeIndex].virtualRegister()];
-
- DataFormat registerFormat = info.registerFormat();
- if (registerFormat != DataFormatNone)
- return (registerFormat | DataFormatJS) == DataFormatJSInteger
- || (registerFormat | DataFormatJS) == DataFormatJSDouble;
-
- DataFormat spillFormat = info.spillFormat();
- if (spillFormat != DataFormatNone)
- return (spillFormat | DataFormatJS) == DataFormatJSInteger
- || (spillFormat | DataFormatJS) == DataFormatJSDouble;
-
- ASSERT(isConstant(nodeIndex));
- return false;
-}
-
void NonSpeculativeJIT::knownConstantArithOp(NodeType op, NodeIndex regChild, NodeIndex immChild, bool commute)
{
JSValueOperand regArg(this, regChild);
@@ -342,46 +302,6 @@
jsValueResult(resultGPR, m_compileIndex);
}
-void NonSpeculativeJIT::compare(Node& node, MacroAssembler::RelationalCondition cond, Z_DFGOperation_EJJ helperFunction)
-{
- // FIXME: should do some peephole to fuse compare/branch
-
- JSValueOperand arg1(this, node.child1());
- JSValueOperand arg2(this, node.child2());
- GPRReg arg1GPR = arg1.gpr();
- GPRReg arg2GPR = arg2.gpr();
-
- GPRTemporary result(this, arg2);
- GPRReg resultGPR = result.gpr();
-
- JITCompiler::JumpList slowPath;
-
- if (!isKnownInteger(node.child1()))
- slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg1GPR, GPRInfo::tagTypeNumberRegister));
- if (!isKnownInteger(node.child2()))
- slowPath.append(m_jit.branchPtr(MacroAssembler::Below, arg2GPR, GPRInfo::tagTypeNumberRegister));
-
- m_jit.compare32(cond, arg1GPR, arg2GPR, resultGPR);
-
- JITCompiler::Jump haveResult = m_jit.jump();
-
- slowPath.link(&m_jit);
-
- silentSpillAllRegisters(resultGPR);
- setupStubArguments(arg1GPR, arg2GPR);
- m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
- appendCallWithExceptionCheck(helperFunction);
- m_jit.move(GPRInfo::returnValueGPR, resultGPR);
- silentFillAllRegisters(resultGPR);
-
- m_jit.andPtr(TrustedImm32(static_cast<int32_t>(1)), resultGPR);
-
- haveResult.link(&m_jit);
-
- m_jit.or32(TrustedImm32(ValueFalse), resultGPR);
- jsValueResult(resultGPR, m_compileIndex);
-}
-
void NonSpeculativeJIT::compile(SpeculationCheckIndexIterator& checkIterator, Node& node)
{
// Check for speculation checks from the corresponding instruction in the
@@ -733,27 +653,33 @@
}
case CompareLess:
- compare(node, MacroAssembler::LessThan, operationCompareLess);
+ if (nonSpeculativeCompare(node, MacroAssembler::LessThan, operationCompareLess))
+ return;
break;
case CompareLessEq:
- compare(node, MacroAssembler::LessThanOrEqual, operationCompareLessEq);
+ if (nonSpeculativeCompare(node, MacroAssembler::LessThanOrEqual, operationCompareLessEq))
+ return;
break;
case CompareGreater:
- compare(node, MacroAssembler::GreaterThan, operationCompareGreater);
+ if (nonSpeculativeCompare(node, MacroAssembler::GreaterThan, operationCompareGreater))
+ return;
break;
case CompareGreaterEq:
- compare(node, MacroAssembler::GreaterThanOrEqual, operationCompareGreaterEq);
+ if (nonSpeculativeCompare(node, MacroAssembler::GreaterThanOrEqual, operationCompareGreaterEq))
+ return;
break;
case CompareEq:
- compare(node, MacroAssembler::Equal, operationCompareEq);
+ if (nonSpeculativeCompare(node, MacroAssembler::Equal, operationCompareEq))
+ return;
break;
case CompareStrictEq:
- compare(node, MacroAssembler::Equal, operationCompareStrictEq);
+ if (nonSpeculativeCompare(node, MacroAssembler::Equal, operationCompareStrictEq))
+ return;
break;
case GetByVal: {
Modified: trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.h (91098 => 91099)
--- trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.h 2011-07-15 20:10:07 UTC (rev 91098)
+++ trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.h 2011-07-15 20:14:27 UTC (rev 91099)
@@ -82,9 +82,6 @@
void compile(SpeculationCheckIndexIterator&, Node&);
void compile(SpeculationCheckIndexIterator&, BasicBlock&);
- bool isKnownInteger(NodeIndex);
- bool isKnownNumeric(NodeIndex);
-
// These methods are used to plant calls out to C++
// helper routines to convert between types.
void valueToNumber(JSValueOperand&, GPRReg result);
@@ -102,7 +99,6 @@
// internal helpers for add/sub/mul operations
void knownConstantArithOp(NodeType op, NodeIndex regChild, NodeIndex immChild, bool commute);
void basicArithOp(NodeType op, Node&);
- void compare(Node&, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ);
EntryLocationVector m_entryLocations;
};
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (91098 => 91099)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-07-15 20:10:07 UTC (rev 91098)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-07-15 20:14:27 UTC (rev 91099)
@@ -424,7 +424,7 @@
if (shouldSpeculateInteger(node.child1(), node.child2()))
compilePeepHoleIntegerBranch(node, branchNodeIndex, condition);
else
- compilePeepHoleCall(node, branchNodeIndex, operation);
+ nonSpeculativePeepholeBranch(node, branchNodeIndex, condition, operation);
use(node.child1());
use(node.child2());
@@ -797,18 +797,10 @@
return;
break;
- case CompareStrictEq: {
- SpeculateIntegerOperand op1(this, node.child1());
- SpeculateIntegerOperand op2(this, node.child2());
- GPRTemporary result(this, op1, op2);
-
- m_jit.compare32(JITCompiler::Equal, op1.gpr(), op2.gpr(), result.gpr());
-
- // If we add a DataFormatBool, we should use it here.
- m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
- jsValueResult(result.gpr(), m_compileIndex);
+ case CompareStrictEq:
+ if (compare(node, JITCompiler::Equal, operationCompareStrictEq))
+ return;
break;
- }
case GetByVal: {
NodeIndex alias = node.child3();
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (91098 => 91099)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-07-15 20:10:07 UTC (rev 91098)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-07-15 20:14:27 UTC (rev 91099)
@@ -139,22 +139,6 @@
void checkArgumentTypes();
void initializeVariableTypes();
- // Returns the node index of the branch node if peephole is okay, NoNode otherwise.
- NodeIndex detectPeepHoleBranch()
- {
- NodeIndex lastNodeIndex = m_jit.graph().m_blocks[m_block]->end - 1;
-
- // Check that no intervening nodes will be generated.
- for (NodeIndex index = m_compileIndex + 1; index < lastNodeIndex; ++index) {
- if (m_jit.graph()[index].shouldGenerate())
- return NoNode;
- }
-
- // Check if the lastNode is a branch on this node.
- Node& lastNode = m_jit.graph()[lastNodeIndex];
- return lastNode.op == Branch && lastNode.child1() == m_compileIndex ? lastNodeIndex : NoNode;
- }
-
bool isInteger(NodeIndex nodeIndex)
{
Node& node = m_jit.graph()[nodeIndex];
