Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (97117 => 97118)
--- trunk/Source/_javascript_Core/ChangeLog 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-10-11 01:14:13 UTC (rev 97118)
@@ -1,3 +1,40 @@
+2011-10-10 Filip Pizlo <fpi...@apple.com>
+
+ DFG JSVALUE64 spill/fill code should not box integers and doubles
+ https://bugs.webkit.org/show_bug.cgi?id=69782
+
+ Reviewed by Oliver Hunt.
+
+ Added the notion of DataFormatInteger and DataFormatDouble to the spillFormat.
+ This required changing all of the places that spill registers (both silently
+ and not) and filling registers (both silently and on demand). It also required
+ changing OSR exit to recognize that a spilled value (DisplacedInRegisterFile)
+ may have the wrong format for the old JIT (unboxed int or double).
+
+ This is a slight win on Kraken (0.25%) and neutral elsewhere.
+
+ * dfg/DFGGenerationInfo.h:
+ (JSC::DFG::GenerationInfo::spill):
+ * dfg/DFGJITCodeGenerator.h:
+ (JSC::DFG::JITCodeGenerator::silentFillFPR):
+ (JSC::DFG::JITCodeGenerator::spill):
+ * dfg/DFGJITCodeGenerator64.cpp:
+ (JSC::DFG::JITCodeGenerator::fillInteger):
+ (JSC::DFG::JITCodeGenerator::fillDouble):
+ (JSC::DFG::JITCodeGenerator::fillJSValue):
+ * dfg/DFGJITCompiler.cpp:
+ (JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::computeValueRecoveryFor):
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::ValueRecovery::displacedInRegisterFile):
+ (JSC::DFG::ValueRecovery::virtualRegister):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::fillSpeculateIntInternal):
+ (JSC::DFG::SpeculativeJIT::fillSpeculateDouble):
+ (JSC::DFG::SpeculativeJIT::fillSpeculateCell):
+ (JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
+
2011-10-10 Gavin Barraclough <baraclo...@apple.com>
DFG JIT switch dfgConvert methods to use callOperation
Modified: trunk/Source/_javascript_Core/dfg/DFGGenerationInfo.h (97117 => 97118)
--- trunk/Source/_javascript_Core/dfg/DFGGenerationInfo.h 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/dfg/DFGGenerationInfo.h 2011-10-11 01:14:13 UTC (rev 97118)
@@ -367,7 +367,7 @@
// pointers. This is not true anymore, but we still assume, in the fill code,
// that any spill slot for a JS value is boxed. For storage pointers, there is
// nothing we can do to box them, so we allow that to be an exception.
- ASSERT((spillFormat & DataFormatJS) || spillFormat == DataFormatStorage);
+ ASSERT((spillFormat & DataFormatJS) || spillFormat == DataFormatStorage || spillFormat == DataFormatInteger || spillFormat == DataFormatDouble);
m_registerFormat = DataFormatNone;
m_spillFormat = spillFormat;
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h (97117 => 97118)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-10-11 01:14:13 UTC (rev 97118)
@@ -383,8 +383,8 @@
return;
}
- if (info.spillFormat() != DataFormatNone) {
- // it was already spilled previously, which means we need unboxing.
+ if (info.spillFormat() != DataFormatNone && info.spillFormat() != DataFormatDouble) {
+ // it was already spilled previously and not as a double, which means we need unboxing.
ASSERT(info.spillFormat() & DataFormatJS);
m_jit.loadPtr(JITCompiler::addressFor(spillMe), canTrample);
unboxDouble(canTrample, info.fpr());
@@ -506,17 +506,20 @@
#if USE(JSVALUE64)
case DataFormatDouble: {
- // All values are spilled as JSValues, so box the double via a temporary gpr.
- GPRReg gpr = boxDouble(info.fpr());
- m_jit.storePtr(gpr, JITCompiler::addressFor(spillMe));
- unlock(gpr);
- info.spill(DataFormatJSDouble);
+ m_jit.storeDouble(info.fpr(), JITCompiler::addressFor(spillMe));
+ info.spill(DataFormatDouble);
return;
}
+
+ case DataFormatInteger: {
+ m_jit.store32(info.gpr(), JITCompiler::payloadFor(spillMe));
+ info.spill(DataFormatInteger);
+ return;
+ }
default:
// The following code handles JSValues, int32s, and cells.
- ASSERT(spillFormat == DataFormatInteger || spillFormat == DataFormatCell || spillFormat & DataFormatJS);
+ ASSERT(spillFormat == DataFormatCell || spillFormat & DataFormatJS);
GPRReg reg = info.gpr();
// We need to box int32 and cell values ...
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator64.cpp (97117 => 97118)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator64.cpp 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator64.cpp 2011-10-11 01:14:13 UTC (rev 97118)
@@ -61,6 +61,11 @@
JSValue jsValue = valueOfJSConstant(nodeIndex);
m_jit.move(MacroAssembler::ImmPtr(JSValue::encode(jsValue)), gpr);
}
+ } else if (info.spillFormat() == DataFormatInteger) {
+ m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
+ m_jit.load32(JITCompiler::payloadFor(virtualRegister), gpr);
+ // Tag it, since fillInteger() is used when we want a boxed integer.
+ m_jit.orPtr(GPRInfo::tagTypeNumberRegister, gpr);
} else {
ASSERT(info.spillFormat() == DataFormatJS || info.spillFormat() == DataFormatJSInteger);
m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
@@ -116,9 +121,9 @@
GenerationInfo& info = m_generationInfo[virtualRegister];
if (info.registerFormat() == DataFormatNone) {
- GPRReg gpr = allocate();
-
if (node.hasConstant()) {
+ GPRReg gpr = allocate();
+
if (isInt32Constant(nodeIndex)) {
// FIXME: should not be reachable?
m_jit.move(MacroAssembler::Imm32(valueOfInt32Constant(nodeIndex)), gpr);
@@ -145,11 +150,35 @@
}
} else {
DataFormat spillFormat = info.spillFormat();
- ASSERT(spillFormat & DataFormatJS);
- m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
- m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
- info.fillJSValue(gpr, spillFormat);
- unlock(gpr);
+ switch (spillFormat) {
+ case DataFormatDouble: {
+ FPRReg fpr = fprAllocate();
+ m_jit.loadDouble(JITCompiler::addressFor(virtualRegister), fpr);
+ m_fprs.retain(fpr, virtualRegister, SpillOrderDouble);
+ info.fillDouble(fpr);
+ return fpr;
+ }
+
+ case DataFormatInteger: {
+ GPRReg gpr = allocate();
+
+ m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
+ m_jit.load32(JITCompiler::addressFor(virtualRegister), gpr);
+ info.fillInteger(gpr);
+ unlock(gpr);
+ break;
+ }
+
+ default:
+ GPRReg gpr = allocate();
+
+ ASSERT(spillFormat & DataFormatJS);
+ m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
+ m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
+ info.fillJSValue(gpr, spillFormat);
+ unlock(gpr);
+ break;
+ }
}
}
@@ -263,9 +292,20 @@
m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
} else {
DataFormat spillFormat = info.spillFormat();
- ASSERT(spillFormat & DataFormatJS);
m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
- m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
+ if (spillFormat == DataFormatInteger) {
+ m_jit.load32(JITCompiler::addressFor(virtualRegister), gpr);
+ m_jit.orPtr(GPRInfo::tagTypeNumberRegister, gpr);
+ spillFormat = DataFormatJSInteger;
+ } else {
+ m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
+ if (spillFormat == DataFormatDouble) {
+ // Need to box the double, since we want a JSValue.
+ m_jit.subPtr(GPRInfo::tagTypeNumberRegister, gpr);
+ spillFormat = DataFormatJSDouble;
+ } else
+ ASSERT(spillFormat & DataFormatJS);
+ }
info.fillJSValue(gpr, spillFormat);
}
return gpr;
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCompiler.cpp (97117 => 97118)
--- trunk/Source/_javascript_Core/dfg/DFGJITCompiler.cpp 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCompiler.cpp 2011-10-11 01:14:13 UTC (rev 97118)
@@ -112,6 +112,8 @@
for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
const ValueRecovery& recovery = exit.valueRecovery(index);
switch (recovery.technique()) {
+ case Int32DisplacedInRegisterFile:
+ case DoubleDisplacedInRegisterFile:
case DisplacedInRegisterFile:
numberOfDisplacedVirtualRegisters++;
ASSERT((int)recovery.virtualRegister() >= 0);
@@ -249,17 +251,43 @@
unsigned displacementIndex = 0;
for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
const ValueRecovery& recovery = exit.valueRecovery(index);
- if (recovery.technique() != DisplacedInRegisterFile)
- continue;
- loadPtr(addressFor(recovery.virtualRegister()), GPRInfo::toRegister(displacementIndex++));
+ switch (recovery.technique()) {
+ case DisplacedInRegisterFile:
+ loadPtr(addressFor(recovery.virtualRegister()), GPRInfo::toRegister(displacementIndex++));
+ break;
+
+ case Int32DisplacedInRegisterFile: {
+ GPRReg gpr = GPRInfo::toRegister(displacementIndex++);
+ load32(addressFor(recovery.virtualRegister()), gpr);
+ orPtr(GPRInfo::tagTypeNumberRegister, gpr);
+ break;
+ }
+
+ case DoubleDisplacedInRegisterFile: {
+ GPRReg gpr = GPRInfo::toRegister(displacementIndex++);
+ loadPtr(addressFor(recovery.virtualRegister()), gpr);
+ subPtr(GPRInfo::tagTypeNumberRegister, gpr);
+ break;
+ }
+
+ default:
+ break;
+ }
}
displacementIndex = 0;
for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
const ValueRecovery& recovery = exit.valueRecovery(index);
- if (recovery.technique() != DisplacedInRegisterFile)
- continue;
- storePtr(GPRInfo::toRegister(displacementIndex++), addressFor((VirtualRegister)exit.operandForIndex(index)));
+ switch (recovery.technique()) {
+ case DisplacedInRegisterFile:
+ case Int32DisplacedInRegisterFile:
+ case DoubleDisplacedInRegisterFile:
+ storePtr(GPRInfo::toRegister(displacementIndex++), addressFor((VirtualRegister)exit.operandForIndex(index)));
+ break;
+
+ default:
+ break;
+ }
}
} else {
// FIXME: This should use the shuffling algorithm that we use
@@ -281,19 +309,46 @@
for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
const ValueRecovery& recovery = exit.valueRecovery(index);
- if (recovery.technique() != DisplacedInRegisterFile)
- continue;
- loadPtr(addressFor(recovery.virtualRegister()), GPRInfo::regT0);
- storePtr(GPRInfo::regT0, scratchBuffer + scratchIndex++);
+
+ switch (recovery.technique()) {
+ case DisplacedInRegisterFile:
+ loadPtr(addressFor(recovery.virtualRegister()), GPRInfo::regT0);
+ storePtr(GPRInfo::regT0, scratchBuffer + scratchIndex++);
+ break;
+
+ case Int32DisplacedInRegisterFile: {
+ load32(addressFor(recovery.virtualRegister()), GPRInfo::regT0);
+ orPtr(GPRInfo::tagTypeNumberRegister, GPRInfo::regT0);
+ storePtr(GPRInfo::regT0, scratchBuffer + scratchIndex++);
+ break;
+ }
+
+ case DoubleDisplacedInRegisterFile: {
+ loadPtr(addressFor(recovery.virtualRegister()), GPRInfo::regT0);
+ subPtr(GPRInfo::tagTypeNumberRegister, GPRInfo::regT0);
+ storePtr(GPRInfo::regT0, scratchBuffer + scratchIndex++);
+ break;
+ }
+
+ default:
+ break;
+ }
}
scratchIndex = numberOfPoisonedVirtualRegisters;
for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
const ValueRecovery& recovery = exit.valueRecovery(index);
- if (recovery.technique() != DisplacedInRegisterFile)
- continue;
- loadPtr(scratchBuffer + scratchIndex++, GPRInfo::regT0);
- storePtr(GPRInfo::regT0, addressFor((VirtualRegister)exit.operandForIndex(index)));
+ switch (recovery.technique()) {
+ case DisplacedInRegisterFile:
+ case Int32DisplacedInRegisterFile:
+ case DoubleDisplacedInRegisterFile:
+ loadPtr(scratchBuffer + scratchIndex++, GPRInfo::regT0);
+ storePtr(GPRInfo::regT0, addressFor((VirtualRegister)exit.operandForIndex(index)));
+ break;
+
+ default:
+ break;
+ }
}
ASSERT(scratchIndex == numberOfPoisonedVirtualRegisters + numberOfDisplacedVirtualRegisters);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (97117 => 97118)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-10-11 01:14:13 UTC (rev 97118)
@@ -516,7 +516,7 @@
return ValueRecovery::inGPR(infoPtr->gpr(), infoPtr->registerFormat());
}
if (infoPtr->spillFormat() != DataFormatNone)
- return ValueRecovery::displacedInRegisterFile(static_cast<VirtualRegister>(nodePtr->virtualRegister()));
+ return ValueRecovery::displacedInRegisterFile(static_cast<VirtualRegister>(nodePtr->virtualRegister()), infoPtr->spillFormat());
ASSERT_NOT_REACHED();
return ValueRecovery();
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (97117 => 97118)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-10-11 01:14:13 UTC (rev 97118)
@@ -158,6 +158,9 @@
InFPR,
// It's in the register file, but at a different location.
DisplacedInRegisterFile,
+ // It's in the register file, at a different location, and it's unboxed.
+ Int32DisplacedInRegisterFile,
+ DoubleDisplacedInRegisterFile,
// It's a constant.
Constant,
// Don't know how to recover it.
@@ -226,10 +229,23 @@
return result;
}
- static ValueRecovery displacedInRegisterFile(VirtualRegister virtualReg)
+ static ValueRecovery displacedInRegisterFile(VirtualRegister virtualReg, DataFormat dataFormat)
{
ValueRecovery result;
- result.m_technique = DisplacedInRegisterFile;
+ switch (dataFormat) {
+ case DataFormatInteger:
+ result.m_technique = Int32DisplacedInRegisterFile;
+ break;
+
+ case DataFormatDouble:
+ result.m_technique = DoubleDisplacedInRegisterFile;
+ break;
+
+ default:
+ ASSERT(dataFormat != DataFormatNone && dataFormat != DataFormatStorage);
+ result.m_technique = DisplacedInRegisterFile;
+ break;
+ }
result.m_source.virtualReg = virtualReg;
return result;
}
@@ -272,7 +288,7 @@
VirtualRegister virtualRegister() const
{
- ASSERT(m_technique == DisplacedInRegisterFile);
+ ASSERT(m_technique == DisplacedInRegisterFile || m_technique == Int32DisplacedInRegisterFile || m_technique == DoubleDisplacedInRegisterFile);
return m_source.virtualReg;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (97117 => 97118)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-10-11 01:01:30 UTC (rev 97117)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2011-10-11 01:14:13 UTC (rev 97118)
@@ -44,7 +44,7 @@
switch (info.registerFormat()) {
case DataFormatNone: {
- if (node.hasConstant() && !isInt32Constant(nodeIndex)) {
+ if ((node.hasConstant() && !isInt32Constant(nodeIndex)) || info.spillFormat() == DataFormatDouble) {
terminateSpeculativeExecution();
returnFormat = DataFormatInteger;
return allocate();
@@ -62,11 +62,12 @@
}
DataFormat spillFormat = info.spillFormat();
- ASSERT(spillFormat & DataFormatJS);
+ ASSERT((spillFormat & DataFormatJS) || spillFormat == DataFormatInteger);
+
m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
- if (spillFormat == DataFormatJSInteger) {
+ if (spillFormat == DataFormatJSInteger || spillFormat == DataFormatInteger) {
// If we know this was spilled as an integer we can fill without checking.
if (strict) {
m_jit.load32(JITCompiler::addressFor(virtualRegister), gpr);
@@ -74,7 +75,11 @@
returnFormat = DataFormatInteger;
return gpr;
}
- m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
+ if (spillFormat == DataFormatInteger) {
+ m_jit.load32(JITCompiler::addressFor(virtualRegister), gpr);
+ m_jit.orPtr(GPRInfo::tagTypeNumberRegister, gpr);
+ } else
+ m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
info.fillJSValue(gpr, DataFormatJSInteger);
returnFormat = DataFormatJSInteger;
return gpr;
@@ -175,9 +180,9 @@
GenerationInfo& info = m_generationInfo[virtualRegister];
if (info.registerFormat() == DataFormatNone) {
- GPRReg gpr = allocate();
+ if (node.hasConstant()) {
+ GPRReg gpr = allocate();
- if (node.hasConstant()) {
if (isInt32Constant(nodeIndex)) {
FPRReg fpr = fprAllocate();
m_jit.move(MacroAssembler::ImmPtr(reinterpret_cast<void*>(reinterpretDoubleToIntptr(static_cast<double>(valueOfInt32Constant(nodeIndex))))), gpr);
@@ -203,11 +208,35 @@
}
DataFormat spillFormat = info.spillFormat();
- ASSERT(spillFormat & DataFormatJS);
- m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
- m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
- info.fillJSValue(gpr, spillFormat);
- unlock(gpr);
+ switch (spillFormat) {
+ case DataFormatDouble: {
+ FPRReg fpr = fprAllocate();
+ m_jit.loadDouble(JITCompiler::addressFor(virtualRegister), fpr);
+ m_fprs.retain(fpr, virtualRegister, SpillOrderDouble);
+ info.fillDouble(fpr);
+ return fpr;
+ }
+
+ case DataFormatInteger: {
+ GPRReg gpr = allocate();
+
+ m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
+ m_jit.load32(JITCompiler::addressFor(virtualRegister), gpr);
+ info.fillInteger(gpr);
+ unlock(gpr);
+ break;
+ }
+
+ default:
+ GPRReg gpr = allocate();
+
+ ASSERT(spillFormat & DataFormatJS);
+ m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
+ m_jit.loadPtr(JITCompiler::addressFor(virtualRegister), gpr);
+ info.fillJSValue(gpr, spillFormat);
+ unlock(gpr);
+ break;
+ }
}
switch (info.registerFormat()) {
@@ -300,6 +329,11 @@
switch (info.registerFormat()) {
case DataFormatNone: {
+ if (info.spillFormat() == DataFormatInteger || info.spillFormat() == DataFormatDouble) {
+ terminateSpeculativeExecution();
+ return allocate();
+ }
+
GPRReg gpr = allocate();
if (node.hasConstant()) {
@@ -368,6 +402,11 @@
switch (info.registerFormat()) {
case DataFormatNone: {
+ if (info.spillFormat() == DataFormatInteger || info.spillFormat() == DataFormatDouble) {
+ terminateSpeculativeExecution();
+ return allocate();
+ }
+
GPRReg gpr = allocate();
if (node.hasConstant()) {
