Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (96660 => 96661)
--- trunk/Source/_javascript_Core/ChangeLog 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-10-04 23:02:25 UTC (rev 96661)
@@ -1,3 +1,33 @@
+2011-10-03 Filip Pizlo <fpi...@apple.com>
+
+ JITCodeGenerator should no longer have code that tries too hard
+ to be both speculative and non-speculative
+ https://bugs.webkit.org/show_bug.cgi?id=69321
+
+ Reviewed by Gavin Barraclough.
+
+ Removed m_isSpeculative and speculationCheck() from JITCodeGenerator.
+ This required moving emitBranch() to SpeculativeJIT, since it was
+ the main user of that field and method. Other than trvial clean-ups
+ in emitBranch(), the code is unchanged (and still has some disparity
+ between 64 and 32_64, and still lacks some obvious optimizations).
+
+ * dfg/DFGJITCodeGenerator.cpp:
+ * dfg/DFGJITCodeGenerator.h:
+ (JSC::DFG::JITCodeGenerator::JITCodeGenerator):
+ * dfg/DFGJITCodeGenerator32_64.cpp:
+ (JSC::DFG::JITCodeGenerator::fillDouble):
+ (JSC::DFG::JITCodeGenerator::fillJSValue):
+ * dfg/DFGJITCodeGenerator64.cpp:
+ (JSC::DFG::JITCodeGenerator::fillDouble):
+ (JSC::DFG::JITCodeGenerator::fillJSValue):
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::SpeculativeJIT::SpeculativeJIT):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::emitBranch):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::emitBranch):
+
2011-10-04 David Hyatt <hy...@apple.com>
https://bugs.webkit.org/show_bug.cgi?id=69372
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp (96660 => 96661)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-10-04 23:02:25 UTC (rev 96661)
@@ -377,12 +377,6 @@
return false;
}
-void JITCodeGenerator::speculationCheck(MacroAssembler::Jump jumpToFail)
-{
- ASSERT(m_isSpeculative);
- static_cast<SpeculativeJIT*>(this)->speculationCheck(jumpToFail);
-}
-
#ifndef NDEBUG
static const char* dataFormatString(DataFormat format)
{
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h (96660 => 96661)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-10-04 23:02:25 UTC (rev 96661)
@@ -232,9 +232,8 @@
}
protected:
- JITCodeGenerator(JITCompiler& jit, bool isSpeculative)
+ JITCodeGenerator(JITCompiler& jit)
: m_jit(jit)
- , m_isSpeculative(isSpeculative)
, m_compileIndex(0)
, m_generationInfo(m_jit.codeBlock()->m_numCalleeRegisters)
, m_blockHeads(jit.graph().m_blocks.size())
@@ -778,8 +777,6 @@
void nonSpeculativeNonPeepholeStrictEq(Node&, bool invert = false);
bool nonSpeculativeStrictEq(Node&, bool invert = false);
- void emitBranch(Node&);
-
MacroAssembler::Address addressOfCallData(int idx)
{
return MacroAssembler::Address(GPRInfo::callFrameRegister, (m_jit.codeBlock()->m_numCalleeRegisters + idx) * static_cast<int>(sizeof(Register)));
@@ -799,8 +796,6 @@
void emitCall(Node&);
- void speculationCheck(MacroAssembler::Jump jumpToFail);
-
// Called once a node has completed code generation but prior to setting
// its result, to free up its children. (This must happen prior to setting
// the nodes result, since the node may have the same VirtualRegister as
@@ -1383,17 +1378,6 @@
// The JIT, while also provides MacroAssembler functionality.
JITCompiler& m_jit;
- // This flag is used to distinguish speculative and non-speculative
- // code generation. This is significant when filling spilled values
- // from the RegisterFile. When spilling we attempt to store information
- // as to the type of boxed value being stored (int32, double, cell), and
- // when filling on the speculative path we will retrieve this type info
- // where available. On the non-speculative path, however, we cannot rely
- // on the spill format info, since the a value being loaded might have
- // been spilled by either the speculative or non-speculative paths (where
- // we entered the non-speculative path on an intervening bail-out), and
- // the value may have been boxed differently on the two paths.
- bool m_isSpeculative;
// The current node being generated.
BlockIndex m_block;
NodeIndex m_compileIndex;
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp (96660 => 96661)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp 2011-10-04 23:02:25 UTC (rev 96661)
@@ -136,7 +136,7 @@
m_jit.emitLoad(nodeIndex, tag, payload);
m_gprs.retain(tag, virtualRegister, SpillOrderSpilled);
m_gprs.retain(payload, virtualRegister, SpillOrderSpilled);
- info.fillJSValue(tag, payload, m_isSpeculative ? spillFormat : DataFormatJS);
+ info.fillJSValue(tag, payload, spillFormat);
unlock(tag);
unlock(payload);
}
@@ -242,7 +242,7 @@
m_jit.emitLoad(nodeIndex, tagGPR, payloadGPR);
m_gprs.retain(tagGPR, virtualRegister, SpillOrderSpilled);
m_gprs.retain(payloadGPR, virtualRegister, SpillOrderSpilled);
- info.fillJSValue(tagGPR, payloadGPR, m_isSpeculative ? spillFormat : DataFormatJS);
+ info.fillJSValue(tagGPR, payloadGPR, spillFormat);
}
return true;
@@ -1517,45 +1517,6 @@
jsValueResult(resultTagGPR, resultPayloadGPR, m_compileIndex, DataFormatJSBoolean, UseChildrenCalledExplicitly);
}
-void JITCodeGenerator::emitBranch(Node& node)
-{
- // FIXME: Add fast cases for known Boolean!
- JSValueOperand value(this, node.child1());
- value.fill();
- GPRReg valueTagGPR = value.tagGPR();
- GPRReg valuePayloadGPR = value.payloadGPR();
-
- GPRTemporary result(this);
- GPRReg resultGPR = result.gpr();
-
- use(node.child1());
-
- BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(node.takenBytecodeOffset());
- BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(node.notTakenBytecodeOffset());
-
- JITCompiler::Jump fastPath = m_jit.branch32(JITCompiler::Equal, valueTagGPR, JITCompiler::TrustedImm32(JSValue::Int32Tag));
- JITCompiler::Jump slowPath = m_jit.branch32(JITCompiler::NotEqual, valueTagGPR, JITCompiler::TrustedImm32(JSValue::BooleanTag));
-
- fastPath.link(&m_jit);
- addBranch(m_jit.branchTest32(JITCompiler::Zero, valuePayloadGPR), notTaken);
- addBranch(m_jit.jump(), taken);
-
- slowPath.link(&m_jit);
- silentSpillAllRegisters(resultGPR);
- m_jit.push(valueTagGPR);
- m_jit.push(valuePayloadGPR);
- m_jit.push(GPRInfo::callFrameRegister);
- appendCallWithExceptionCheck(dfgConvertJSValueToBoolean);
- m_jit.move(GPRInfo::returnValueGPR, resultGPR);
- silentFillAllRegisters(resultGPR);
-
- addBranch(m_jit.branchTest8(JITCompiler::NonZero, resultGPR), taken);
- if (notTaken != (m_block + 1))
- addBranch(m_jit.jump(), notTaken);
-
- noResult(m_compileIndex, UseChildrenCalledExplicitly);
-}
-
void JITCodeGenerator::emitCall(Node& node)
{
P_DFGOperation_E slowCallFunction;
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator64.cpp (96660 => 96661)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator64.cpp 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator64.cpp 2011-10-04 23:02:25 UTC (rev 96661)
@@ -148,7 +148,7 @@
ASSERT(spillFormat & DataFormatJS);
m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
- info.fillJSValue(gpr, m_isSpeculative ? spillFormat : DataFormatJS);
+ info.fillJSValue(gpr, spillFormat);
unlock(gpr);
}
}
@@ -266,7 +266,7 @@
ASSERT(spillFormat & DataFormatJS);
m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
- info.fillJSValue(gpr, m_isSpeculative ? spillFormat : DataFormatJS);
+ info.fillJSValue(gpr, spillFormat);
}
return gpr;
}
@@ -1461,70 +1461,6 @@
jsValueResult(resultGPR, m_compileIndex, DataFormatJSBoolean, UseChildrenCalledExplicitly);
}
-void JITCodeGenerator::emitBranch(Node& node)
-{
- JSValueOperand value(this, node.child1());
- GPRReg valueGPR = value.gpr();
-
- BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(node.takenBytecodeOffset());
- BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(node.notTakenBytecodeOffset());
-
- if (isKnownBoolean(node.child1())) {
- MacroAssembler::ResultCondition condition = MacroAssembler::NonZero;
-
- if (taken == (m_block + 1)) {
- condition = MacroAssembler::Zero;
- BlockIndex tmp = taken;
- taken = notTaken;
- notTaken = tmp;
- }
-
- addBranch(m_jit.branchTest32(condition, valueGPR, TrustedImm32(true)), taken);
- if (notTaken != (m_block + 1))
- addBranch(m_jit.jump(), notTaken);
-
- noResult(m_compileIndex);
- } else {
- GPRTemporary result(this);
- GPRReg resultGPR = result.gpr();
-
- bool predictBoolean = isBooleanPrediction(m_jit.getPrediction(node.child1()));
-
- if (predictBoolean) {
- addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(false)))), notTaken);
- addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(true)))), taken);
- }
-
- if (m_isSpeculative && predictBoolean) {
- speculationCheck(m_jit.jump());
- value.use();
- } else {
- addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsNumber(0)))), notTaken);
- addBranch(m_jit.branchPtr(MacroAssembler::AboveOrEqual, valueGPR, GPRInfo::tagTypeNumberRegister), taken);
-
- if (!predictBoolean) {
- addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(false)))), notTaken);
- addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(true)))), taken);
- }
-
- value.use();
-
- silentSpillAllRegisters(resultGPR);
- m_jit.move(valueGPR, GPRInfo::argumentGPR1);
- m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
- appendCallWithExceptionCheck(dfgConvertJSValueToBoolean);
- m_jit.move(GPRInfo::returnValueGPR, resultGPR);
- silentFillAllRegisters(resultGPR);
-
- addBranch(m_jit.branchTest8(MacroAssembler::NonZero, resultGPR), taken);
- if (notTaken != (m_block + 1))
- addBranch(m_jit.jump(), notTaken);
- }
-
- noResult(m_compileIndex, UseChildrenCalledExplicitly);
- }
-}
-
void JITCodeGenerator::emitCall(Node& node)
{
P_DFGOperation_E slowCallFunction;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (96660 => 96661)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-10-04 23:02:25 UTC (rev 96661)
@@ -600,6 +600,7 @@
void compileObjectEquality(Node&, void* vptr);
void compileValueAdd(Node&);
void compileLogicalNot(Node&);
+ void emitBranch(Node&);
// It is acceptable to have structure be equal to scratch, so long as you're fine
// with the structure GPR being clobbered.
@@ -943,7 +944,7 @@
};
inline SpeculativeJIT::SpeculativeJIT(JITCompiler& jit)
- : JITCodeGenerator(jit, true)
+ : JITCodeGenerator(jit)
, m_compileOkay(true)
, m_arguments(jit.codeBlock()->m_numParameters)
, m_variables(jit.graph().m_localVars)
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (96660 => 96661)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2011-10-04 23:02:25 UTC (rev 96661)
@@ -482,6 +482,45 @@
#endif
}
+void SpeculativeJIT::emitBranch(Node& node)
+{
+ // FIXME: Add fast cases for known Boolean!
+ JSValueOperand value(this, node.child1());
+ value.fill();
+ GPRReg valueTagGPR = value.tagGPR();
+ GPRReg valuePayloadGPR = value.payloadGPR();
+
+ GPRTemporary result(this);
+ GPRReg resultGPR = result.gpr();
+
+ use(node.child1());
+
+ BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(node.takenBytecodeOffset());
+ BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(node.notTakenBytecodeOffset());
+
+ JITCompiler::Jump fastPath = m_jit.branch32(JITCompiler::Equal, valueTagGPR, JITCompiler::TrustedImm32(JSValue::Int32Tag));
+ JITCompiler::Jump slowPath = m_jit.branch32(JITCompiler::NotEqual, valueTagGPR, JITCompiler::TrustedImm32(JSValue::BooleanTag));
+
+ fastPath.link(&m_jit);
+ addBranch(m_jit.branchTest32(JITCompiler::Zero, valuePayloadGPR), notTaken);
+ addBranch(m_jit.jump(), taken);
+
+ slowPath.link(&m_jit);
+ silentSpillAllRegisters(resultGPR);
+ m_jit.push(valueTagGPR);
+ m_jit.push(valuePayloadGPR);
+ m_jit.push(GPRInfo::callFrameRegister);
+ appendCallWithExceptionCheck(dfgConvertJSValueToBoolean);
+ m_jit.move(GPRInfo::returnValueGPR, resultGPR);
+ silentFillAllRegisters(resultGPR);
+
+ addBranch(m_jit.branchTest8(JITCompiler::NonZero, resultGPR), taken);
+ if (notTaken != (m_block + 1))
+ addBranch(m_jit.jump(), notTaken);
+
+ noResult(m_compileIndex, UseChildrenCalledExplicitly);
+}
+
void SpeculativeJIT::compile(Node& node)
{
NodeType op = node.op;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (96660 => 96661)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-10-04 23:01:00 UTC (rev 96660)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-10-04 23:02:25 UTC (rev 96661)
@@ -582,6 +582,66 @@
jsValueResult(resultGPR, m_compileIndex, DataFormatJSBoolean, UseChildrenCalledExplicitly);
}
+void SpeculativeJIT::emitBranch(Node& node)
+{
+ JSValueOperand value(this, node.child1());
+ GPRReg valueGPR = value.gpr();
+
+ BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(node.takenBytecodeOffset());
+ BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(node.notTakenBytecodeOffset());
+
+ if (isKnownBoolean(node.child1())) {
+ MacroAssembler::ResultCondition condition = MacroAssembler::NonZero;
+
+ if (taken == (m_block + 1)) {
+ condition = MacroAssembler::Zero;
+ BlockIndex tmp = taken;
+ taken = notTaken;
+ notTaken = tmp;
+ }
+
+ addBranch(m_jit.branchTest32(condition, valueGPR, TrustedImm32(true)), taken);
+ if (notTaken != (m_block + 1))
+ addBranch(m_jit.jump(), notTaken);
+
+ noResult(m_compileIndex);
+ } else {
+ GPRTemporary result(this);
+ GPRReg resultGPR = result.gpr();
+
+ bool predictBoolean = isBooleanPrediction(m_jit.getPrediction(node.child1()));
+
+ if (predictBoolean) {
+ addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(false)))), notTaken);
+ addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(true)))), taken);
+
+ speculationCheck(m_jit.jump());
+ value.use();
+ } else {
+ addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsNumber(0)))), notTaken);
+ addBranch(m_jit.branchPtr(MacroAssembler::AboveOrEqual, valueGPR, GPRInfo::tagTypeNumberRegister), taken);
+
+ addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(false)))), notTaken);
+ addBranch(m_jit.branchPtr(MacroAssembler::Equal, valueGPR, MacroAssembler::ImmPtr(JSValue::encode(jsBoolean(true)))), taken);
+
+ value.use();
+
+ silentSpillAllRegisters(resultGPR);
+ m_jit.move(valueGPR, GPRInfo::argumentGPR1);
+ m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+ appendCallWithExceptionCheck(dfgConvertJSValueToBoolean);
+ m_jit.move(GPRInfo::returnValueGPR, resultGPR);
+ silentFillAllRegisters(resultGPR);
+
+ addBranch(m_jit.branchTest8(MacroAssembler::NonZero, resultGPR), taken);
+ if (notTaken != (m_block + 1))
+ addBranch(m_jit.jump(), notTaken);
+ }
+
+ noResult(m_compileIndex, UseChildrenCalledExplicitly);
+ }
+}
+
void SpeculativeJIT::compile(Node& node)
{
NodeType op = node.op;
