- Revision
- 91010
- Author
- commit-qu...@webkit.org
- Date
- 2011-07-14 10:32:13 -0700 (Thu, 14 Jul 2011)
Log Message
DFG JIT unnecessarily boxes and unboxes values during silent spilling.
https://bugs.webkit.org/show_bug.cgi?id=64068
Patch by Filip Pizlo <fpi...@apple.com> on 2011-07-14
Reviewed by Gavin Barraclough.
Silent spilling and filling of registers is done during slow-path C
function calls. The silent spill/fill logic does not affect register
allocation on paths that don't involve the C function call.
This changes the silent spilling code to spill in unboxed form. The
silent fill will refill in whatever form the register was spilled in.
For example, the silent spill code may choose not to spill the register
because it was already spilled previously, which would imply that it
was spilled in boxed form. The filling code detects this and either
unboxes, or not, depending on what is appropriate.
This change also results in a simplification of the silent spill/fill
API: silent spilling no longer needs to know about the set of registers
that cannot be trampled, since it never does boxing and hence does not
need a temporary register.
* dfg/DFGJITCodeGenerator.cpp:
(JSC::DFG::JITCodeGenerator::cachedGetById):
(JSC::DFG::JITCodeGenerator::cachedPutById):
* dfg/DFGJITCodeGenerator.h:
(JSC::DFG::JITCodeGenerator::silentSpillGPR):
(JSC::DFG::JITCodeGenerator::silentSpillFPR):
(JSC::DFG::JITCodeGenerator::silentFillFPR):
(JSC::DFG::JITCodeGenerator::silentSpillAllRegisters):
* dfg/DFGNonSpeculativeJIT.cpp:
(JSC::DFG::NonSpeculativeJIT::valueToNumber):
(JSC::DFG::NonSpeculativeJIT::valueToInt32):
(JSC::DFG::NonSpeculativeJIT::knownConstantArithOp):
(JSC::DFG::NonSpeculativeJIT::basicArithOp):
(JSC::DFG::NonSpeculativeJIT::compare):
(JSC::DFG::NonSpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compile):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (91009 => 91010)
--- trunk/Source/_javascript_Core/ChangeLog 2011-07-14 17:24:13 UTC (rev 91009)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-07-14 17:32:13 UTC (rev 91010)
@@ -1,3 +1,44 @@
+2011-07-14 Filip Pizlo <fpi...@apple.com>
+
+ DFG JIT unnecessarily boxes and unboxes values during silent spilling.
+ https://bugs.webkit.org/show_bug.cgi?id=64068
+
+ Reviewed by Gavin Barraclough.
+
+ Silent spilling and filling of registers is done during slow-path C
+ function calls. The silent spill/fill logic does not affect register
+ allocation on paths that don't involve the C function call.
+
+ This changes the silent spilling code to spill in unboxed form. The
+ silent fill will refill in whatever form the register was spilled in.
+ For example, the silent spill code may choose not to spill the register
+ because it was already spilled previously, which would imply that it
+ was spilled in boxed form. The filling code detects this and either
+ unboxes, or not, depending on what is appropriate.
+
+ This change also results in a simplification of the silent spill/fill
+ API: silent spilling no longer needs to know about the set of registers
+ that cannot be trampled, since it never does boxing and hence does not
+ need a temporary register.
+
+ * dfg/DFGJITCodeGenerator.cpp:
+ (JSC::DFG::JITCodeGenerator::cachedGetById):
+ (JSC::DFG::JITCodeGenerator::cachedPutById):
+ * dfg/DFGJITCodeGenerator.h:
+ (JSC::DFG::JITCodeGenerator::silentSpillGPR):
+ (JSC::DFG::JITCodeGenerator::silentSpillFPR):
+ (JSC::DFG::JITCodeGenerator::silentFillFPR):
+ (JSC::DFG::JITCodeGenerator::silentSpillAllRegisters):
+ * dfg/DFGNonSpeculativeJIT.cpp:
+ (JSC::DFG::NonSpeculativeJIT::valueToNumber):
+ (JSC::DFG::NonSpeculativeJIT::valueToInt32):
+ (JSC::DFG::NonSpeculativeJIT::knownConstantArithOp):
+ (JSC::DFG::NonSpeculativeJIT::basicArithOp):
+ (JSC::DFG::NonSpeculativeJIT::compare):
+ (JSC::DFG::NonSpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+
2011-07-13 Michael Saboff <msab...@apple.com>
https://bugs.webkit.org/show_bug.cgi?id=64202
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp (91009 => 91010)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-07-14 17:24:13 UTC (rev 91009)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-07-14 17:32:13 UTC (rev 91010)
@@ -349,7 +349,7 @@
JITCompiler::Label slowCase = m_jit.label();
- silentSpillAllRegisters(resultGPR, baseGPR);
+ silentSpillAllRegisters(resultGPR);
m_jit.move(baseGPR, GPRInfo::argumentGPR1);
m_jit.move(JITCompiler::ImmPtr(identifier(identifierNumber)), GPRInfo::argumentGPR2);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
@@ -414,7 +414,7 @@
JITCompiler::Label slowCase = m_jit.label();
- silentSpillAllRegisters(InvalidGPRReg, baseGPR, valueGPR);
+ silentSpillAllRegisters(InvalidGPRReg);
setupTwoStubArgs<GPRInfo::argumentGPR1, GPRInfo::argumentGPR2>(valueGPR, baseGPR);
m_jit.move(JITCompiler::ImmPtr(identifier(identifierNumber)), GPRInfo::argumentGPR3);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h (91009 => 91010)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-07-14 17:24:13 UTC (rev 91009)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-07-14 17:32:13 UTC (rev 91010)
@@ -188,7 +188,7 @@
// they spill all live values to the appropriate
// slots in the RegisterFile without changing any state
// in the GenerationInfo.
- void silentSpillGPR(VirtualRegister spillMe, GPRReg canTrample, GPRReg exclude = InvalidGPRReg)
+ void silentSpillGPR(VirtualRegister spillMe, GPRReg exclude = InvalidGPRReg)
{
GenerationInfo& info = m_generationInfo[spillMe];
ASSERT(info.registerFormat() != DataFormatNone);
@@ -200,23 +200,30 @@
DataFormat registerFormat = info.registerFormat();
if (registerFormat == DataFormatInteger) {
- m_jit.orPtr(GPRInfo::tagTypeNumberRegister, info.gpr(), canTrample);
- m_jit.storePtr(canTrample, JITCompiler::addressFor(spillMe));
+ m_jit.store32(info.gpr(), JITCompiler::addressFor(spillMe));
} else {
ASSERT(registerFormat & DataFormatJS || registerFormat == DataFormatCell);
m_jit.storePtr(info.gpr(), JITCompiler::addressFor(spillMe));
}
}
- void silentSpillFPR(VirtualRegister spillMe, GPRReg canTrample, FPRReg exclude = InvalidFPRReg)
+ void silentSpillFPR(VirtualRegister spillMe, FPRReg exclude = InvalidFPRReg)
{
GenerationInfo& info = m_generationInfo[spillMe];
ASSERT(info.registerFormat() == DataFormatDouble);
- if (!info.needsSpill() || (info.fpr() == exclude))
+ if (info.fpr() == exclude)
return;
+ if (!info.needsSpill()) {
+ // it's either a constant or it's already been spilled
+ ASSERT(m_jit.graph()[info.nodeIndex()].isConstant() || info.spillFormat() != DataFormatNone);
+ return;
+ }
+
+ // it's neither a constant nor has it been spilled.
+ ASSERT(!m_jit.graph()[info.nodeIndex()].isConstant());
+ ASSERT(info.spillFormat() == DataFormatNone);
- boxDouble(info.fpr(), canTrample);
- m_jit.storePtr(canTrample, JITCompiler::addressFor(spillMe));
+ m_jit.storeDouble(info.fpr(), JITCompiler::addressFor(spillMe));
}
void silentFillGPR(VirtualRegister spillMe, GPRReg exclude = InvalidGPRReg)
@@ -261,38 +268,40 @@
ASSERT(isDoubleConstant(nodeIndex));
m_jit.move(JITCompiler::ImmPtr(bitwise_cast<void*>(valueOfDoubleConstant(nodeIndex))), canTrample);
m_jit.movePtrToDouble(canTrample, info.fpr());
- } else {
+ return;
+ }
+
+ if (info.spillFormat() != DataFormatNone) {
+ // it was already spilled previously, which means we need unboxing.
+ ASSERT(info.spillFormat() & DataFormatJS);
m_jit.loadPtr(JITCompiler::addressFor(spillMe), canTrample);
unboxDouble(canTrample, info.fpr());
+ return;
}
+
+ m_jit.loadDouble(JITCompiler::addressFor(spillMe), info.fpr());
}
- void silentSpillAllRegisters(GPRReg exclude, GPRReg preserve1 = InvalidGPRReg, GPRReg preserve2 = InvalidGPRReg, GPRReg preserve3 = InvalidGPRReg)
+ void silentSpillAllRegisters(GPRReg exclude)
{
- GPRReg canTrample = selectScratchGPR(preserve1, preserve2, preserve3);
-
for (gpr_iterator iter = m_gprs.begin(); iter != m_gprs.end(); ++iter) {
if (iter.name() != InvalidVirtualRegister)
- silentSpillGPR(iter.name(), canTrample, exclude);
+ silentSpillGPR(iter.name(), exclude);
}
for (fpr_iterator iter = m_fprs.begin(); iter != m_fprs.end(); ++iter) {
if (iter.name() != InvalidVirtualRegister)
- silentSpillFPR(iter.name(), canTrample);
+ silentSpillFPR(iter.name());
}
}
- void silentSpillAllRegisters(FPRReg exclude, GPRReg preserve = InvalidGPRReg)
+ void silentSpillAllRegisters(FPRReg exclude)
{
- GPRReg canTrample = GPRInfo::regT0;
- if (preserve == GPRInfo::regT0)
- canTrample = GPRInfo::regT1;
-
for (gpr_iterator iter = m_gprs.begin(); iter != m_gprs.end(); ++iter) {
if (iter.name() != InvalidVirtualRegister)
- silentSpillGPR(iter.name(), canTrample);
+ silentSpillGPR(iter.name());
}
for (fpr_iterator iter = m_fprs.begin(); iter != m_fprs.end(); ++iter) {
if (iter.name() != InvalidVirtualRegister)
- silentSpillFPR(iter.name(), canTrample, exclude);
+ silentSpillFPR(iter.name(), exclude);
}
}
void silentFillAllRegisters(GPRReg exclude)
Modified: trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp (91009 => 91010)
--- trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-07-14 17:24:13 UTC (rev 91009)
+++ trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-07-14 17:32:13 UTC (rev 91010)
@@ -69,7 +69,7 @@
// Next handle cells (& other JS immediates)
nonNumeric.link(&m_jit);
- silentSpillAllRegisters(gpr, jsValueGpr);
+ silentSpillAllRegisters(gpr);
m_jit.move(jsValueGpr, GPRInfo::argumentGPR1);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
appendCallWithExceptionCheck(dfgConvertJSValueToNumber);
@@ -91,7 +91,7 @@
JITCompiler::Jump isInteger = m_jit.branchPtr(MacroAssembler::AboveOrEqual, jsValueGpr, GPRInfo::tagTypeNumberRegister);
// First handle non-integers
- silentSpillAllRegisters(result, jsValueGpr);
+ silentSpillAllRegisters(result);
m_jit.move(jsValueGpr, GPRInfo::argumentGPR1);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
appendCallWithExceptionCheck(dfgConvertJSValueToInt32);
@@ -220,7 +220,7 @@
if (!isKnownInteger(regChild))
notInt.link(&m_jit);
- silentSpillAllRegisters(resultGPR, regArgGPR);
+ silentSpillAllRegisters(resultGPR);
switch (op) {
case ValueAdd:
if (commute) {
@@ -310,7 +310,7 @@
slowPath.link(&m_jit);
- silentSpillAllRegisters(resultGPR, arg1GPR, arg2GPR);
+ silentSpillAllRegisters(resultGPR);
if (op == ValueAdd) {
setupStubArguments(arg1GPR, arg2GPR);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
@@ -367,7 +367,7 @@
slowPath.link(&m_jit);
- silentSpillAllRegisters(resultGPR, arg1GPR, arg2GPR);
+ silentSpillAllRegisters(resultGPR);
setupStubArguments(arg1GPR, arg2GPR);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
appendCallWithExceptionCheck(helperFunction);
@@ -718,7 +718,7 @@
m_jit.xorPtr(TrustedImm32(static_cast<int32_t>(ValueFalse)), resultGPR);
JITCompiler::Jump fastCase = m_jit.branchTestPtr(JITCompiler::Zero, resultGPR, TrustedImm32(static_cast<int32_t>(~1)));
- silentSpillAllRegisters(resultGPR, arg1GPR);
+ silentSpillAllRegisters(resultGPR);
m_jit.move(arg1GPR, GPRInfo::argumentGPR1);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
appendCallWithExceptionCheck(dfgConvertJSValueToBoolean);
@@ -794,7 +794,7 @@
outOfBounds.link(&m_jit);
loadFailed.link(&m_jit);
- silentSpillAllRegisters(storageGPR, baseGPR, propertyGPR);
+ silentSpillAllRegisters(storageGPR);
setupStubArguments(baseGPR, propertyGPR);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
appendCallWithExceptionCheck(operationGetByVal);
@@ -1038,7 +1038,7 @@
notDefaultHasInstance.link(&m_jit);
protoNotObject.link(&m_jit);
- silentSpillAllRegisters(scratchReg, valueReg, baseReg, prototypeReg);
+ silentSpillAllRegisters(scratchReg);
setupStubArguments(valueReg, baseReg, prototypeReg);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
appendCallWithExceptionCheck(operationInstanceOf);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (91009 => 91010)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-07-14 17:24:13 UTC (rev 91009)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-07-14 17:32:13 UTC (rev 91010)
@@ -876,7 +876,7 @@
MacroAssembler::Jump withinArrayBounds = m_jit.branch32(MacroAssembler::Below, propertyReg, MacroAssembler::Address(baseReg, JSArray::vectorLengthOffset()));
// Code to handle put beyond array bounds.
- silentSpillAllRegisters(scratchReg, baseReg, propertyReg, valueReg);
+ silentSpillAllRegisters(scratchReg);
setupStubArguments(baseReg, propertyReg, valueReg);
m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
JITCompiler::Call functionCall = appendCallWithExceptionCheck(operationPutByValBeyondArrayBounds);
