Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (284922 => 284923)
--- trunk/Source/_javascript_Core/ChangeLog 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-10-27 15:34:42 UTC (rev 284923)
@@ -1,3 +1,63 @@
+2021-10-27 Geza Lore <[email protected]>
+
+ [JSC][32bit] Fix CSR restore on DFG tail calls, add extra register on ARMv7
+ https://bugs.webkit.org/show_bug.cgi?id=230622
+
+ Reviewed by Keith Miller.
+
+ This re-introduces the patch reverted by
+ https://trac.webkit.org/changeset/284911/webkit
+ with the C_LOOP interpreter now fixed.
+
+ The only difference between the original patch and this version is in
+ LowLevelInterpreter32_64.asm and LowLevelInterpreter64.asm, which
+ need the PC base (PB) register restored on C_LOOP on return from a
+ call, as C_LOOP does not seem to handle this as a proper callee save
+ register (CSR). On non C_LOOP builds, the CSR restore mechanism takes
+ care of this, so removed the superfluous instructions.
+
+ --- Original ChangeLog ---
+
+ This patch does two things:
+
+ 1. Adds an extra callee save register (CSR) to be available to DFG on
+ ARMv7. To do this properly required the following:
+
+ 2. Implements the necessary shuffling in CallFrameShuffler on 32-bit
+ architectures that is required to restore CSRs properly after a tail
+ call on these architectures. This also fixes the remaining failures in
+ the 32-bit build of the unlinked baseline JIT.
+
+ * bytecode/ValueRecovery.cpp:
+ (JSC::ValueRecovery::dumpInContext const):
+ * bytecode/ValueRecovery.h:
+ (JSC::ValueRecovery::calleeSaveRegDisplacedInJSStack):
+ (JSC::ValueRecovery::isInJSStack const):
+ (JSC::ValueRecovery::dataFormat const):
+ (JSC::ValueRecovery::withLocalsOffset const):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::emitCall):
+ * jit/CachedRecovery.cpp:
+ (JSC::CachedRecovery::loadsIntoGPR const):
+ * jit/CallFrameShuffleData.cpp:
+ (JSC::CallFrameShuffleData::setupCalleeSaveRegisters):
+ * jit/CallFrameShuffleData.h:
+ * jit/CallFrameShuffler.cpp:
+ (JSC::CallFrameShuffler::CallFrameShuffler):
+ * jit/CallFrameShuffler.h:
+ (JSC::CallFrameShuffler::snapshot const):
+ (JSC::CallFrameShuffler::addNew):
+ * jit/CallFrameShuffler32_64.cpp:
+ (JSC::CallFrameShuffler::emitLoad):
+ (JSC::CallFrameShuffler::emitDisplace):
+ * jit/GPRInfo.h:
+ (JSC::GPRInfo::toRegister):
+ (JSC::GPRInfo::toIndex):
+ * jit/RegisterSet.cpp:
+ (JSC::RegisterSet::dfgCalleeSaveRegisters):
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+
2021-10-26 Commit Queue <[email protected]>
Unreviewed, reverting r284255.
Modified: trunk/Source/_javascript_Core/bytecode/ValueRecovery.cpp (284922 => 284923)
--- trunk/Source/_javascript_Core/bytecode/ValueRecovery.cpp 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/bytecode/ValueRecovery.cpp 2021-10-27 15:34:42 UTC (rev 284923)
@@ -99,6 +99,11 @@
case Int32DisplacedInJSStack:
out.print("*int32(", virtualRegister(), ")");
return;
+#if USE(JSVALUE32_64)
+ case Int32TagDisplacedInJSStack:
+ out.print("*int32Tag(", virtualRegister(), ")");
+ return;
+#endif
case Int52DisplacedInJSStack:
out.print("*int52(", virtualRegister(), ")");
return;
Modified: trunk/Source/_javascript_Core/bytecode/ValueRecovery.h (284922 => 284923)
--- trunk/Source/_javascript_Core/bytecode/ValueRecovery.h 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/bytecode/ValueRecovery.h 2021-10-27 15:34:42 UTC (rev 284923)
@@ -60,6 +60,9 @@
DisplacedInJSStack,
// It's in the stack, at a different location, and it's unboxed.
Int32DisplacedInJSStack,
+#if USE(JSVALUE32_64)
+ Int32TagDisplacedInJSStack, // int32 stored in tag field
+#endif
Int52DisplacedInJSStack,
StrictInt52DisplacedInJSStack,
DoubleDisplacedInJSStack,
@@ -187,7 +190,19 @@
result.m_source = WTFMove(u);
return result;
}
-
+
+#if USE(JSVALUE32_64)
+ static ValueRecovery calleeSaveRegDisplacedInJSStack(VirtualRegister virtualReg, bool inTag)
+ {
+ ValueRecovery result;
+ UnionType u;
+ u.virtualReg = virtualReg.offset();
+ result.m_source = WTFMove(u);
+ result.m_technique = inTag ? Int32TagDisplacedInJSStack : Int32DisplacedInJSStack;
+ return result;
+ }
+#endif
+
static ValueRecovery constant(JSValue value)
{
ValueRecovery result;
@@ -258,6 +273,9 @@
switch (m_technique) {
case DisplacedInJSStack:
case Int32DisplacedInJSStack:
+#if USE(JSVALUE32_64)
+ case Int32TagDisplacedInJSStack:
+#endif
case Int52DisplacedInJSStack:
case StrictInt52DisplacedInJSStack:
case DoubleDisplacedInJSStack:
@@ -282,6 +300,9 @@
return DataFormatJS;
case UnboxedInt32InGPR:
case Int32DisplacedInJSStack:
+#if USE(JSVALUE32_64)
+ case Int32TagDisplacedInJSStack:
+#endif
return DataFormatInt32;
case UnboxedInt52InGPR:
case Int52DisplacedInJSStack:
@@ -358,6 +379,9 @@
switch (m_technique) {
case DisplacedInJSStack:
case Int32DisplacedInJSStack:
+#if USE(JSVALUE32_64)
+ case Int32TagDisplacedInJSStack:
+#endif
case DoubleDisplacedInJSStack:
case CellDisplacedInJSStack:
case BooleanDisplacedInJSStack:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (284922 => 284923)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2021-10-27 15:34:42 UTC (rev 284923)
@@ -742,6 +742,8 @@
for (unsigned i = numPassedArgs; i < numAllocatedArgs; ++i)
shuffleData.args[i] = ValueRecovery::constant(jsUndefined());
+
+ shuffleData.setupCalleeSaveRegisters(m_jit.codeBlock());
} else {
m_jit.store32(MacroAssembler::TrustedImm32(numPassedArgs), m_jit.calleeFramePayloadSlot(CallFrameSlot::argumentCountIncludingThis));
Modified: trunk/Source/_javascript_Core/jit/CachedRecovery.cpp (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/CachedRecovery.cpp 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/CachedRecovery.cpp 2021-10-27 15:34:42 UTC (rev 284923)
@@ -52,7 +52,9 @@
{
switch (recovery().technique()) {
case Int32DisplacedInJSStack:
-#if USE(JSVALUE64)
+#if USE(JSVALUE32_64)
+ case Int32TagDisplacedInJSStack:
+#elif USE(JSVALUE64)
case Int52DisplacedInJSStack:
case StrictInt52DisplacedInJSStack:
case DisplacedInJSStack:
Modified: trunk/Source/_javascript_Core/jit/CallFrameShuffleData.cpp (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/CallFrameShuffleData.cpp 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/CallFrameShuffleData.cpp 2021-10-27 15:34:42 UTC (rev 284923)
@@ -33,8 +33,6 @@
namespace JSC {
-#if USE(JSVALUE64)
-
void CallFrameShuffleData::setupCalleeSaveRegisters(CodeBlock* codeBlock)
{
setupCalleeSaveRegisters(codeBlock->calleeSaveRegisters());
@@ -49,9 +47,24 @@
if (!calleeSaveRegisters.get(entry.reg()))
continue;
- VirtualRegister saveSlot { entry.offsetAsIndex() };
+ int saveSlotIndexInCPURegisters = entry.offsetAsIndex();
+
+#if USE(JSVALUE64)
+ // CPU registers are the same size as virtual registers
+ VirtualRegister saveSlot { saveSlotIndexInCPURegisters };
registers[entry.reg()]
= ValueRecovery::displacedInJSStack(saveSlot, DataFormatJS);
+#elif USE(JSVALUE32_64)
+ // On 32-bit architectures, 2 callee saved registers may be packed into the same slot
+ static_assert(!PayloadOffset || !TagOffset);
+ static_assert(PayloadOffset == 4 || TagOffset == 4);
+ bool inTag = (saveSlotIndexInCPURegisters & 1) == !!TagOffset;
+ if (saveSlotIndexInCPURegisters < 0)
+ saveSlotIndexInCPURegisters -= 1; // Round towards -inf
+ VirtualRegister saveSlot { saveSlotIndexInCPURegisters / 2 };
+ registers[entry.reg()]
+ = ValueRecovery::calleeSaveRegDisplacedInJSStack(saveSlot, inTag);
+#endif
}
for (Reg reg = Reg::first(); reg <= Reg::last(); reg = reg.next()) {
@@ -61,12 +74,14 @@
if (registers[reg])
continue;
+#if USE(JSVALUE64)
registers[reg] = ValueRecovery::inRegister(reg, DataFormatJS);
+#elif USE(JSVALUE32_64)
+ registers[reg] = ValueRecovery::inRegister(reg, DataFormatInt32);
+#endif
}
}
-#endif // USE(JSVALUE64)
-
} // namespace JSC
#endif // ENABLE(JIT)
Modified: trunk/Source/_javascript_Core/jit/CallFrameShuffleData.h (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/CallFrameShuffleData.h 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/CallFrameShuffleData.h 2021-10-27 15:34:42 UTC (rev 284923)
@@ -44,13 +44,13 @@
unsigned numLocals { UINT_MAX };
unsigned numPassedArgs { UINT_MAX };
unsigned numParameters { UINT_MAX }; // On our machine frame.
+ RegisterMap<ValueRecovery> registers;
#if USE(JSVALUE64)
- RegisterMap<ValueRecovery> registers;
GPRReg numberTagRegister { InvalidGPRReg };
+#endif
void setupCalleeSaveRegisters(CodeBlock*);
void setupCalleeSaveRegisters(const RegisterAtOffsetList*);
-#endif
ValueRecovery callee;
};
Modified: trunk/Source/_javascript_Core/jit/CallFrameShuffler.cpp (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/CallFrameShuffler.cpp 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/CallFrameShuffler.cpp 2021-10-27 15:34:42 UTC (rev 284923)
@@ -52,13 +52,8 @@
for (unsigned i = FPRInfo::numberOfRegisters; i--; )
m_lockedRegisters.clear(FPRInfo::toRegister(i));
-#if USE(JSVALUE64)
- // ... as well as the runtime registers on 64-bit architectures.
- // However do not use these registers on 32-bit architectures since
- // saving and restoring callee-saved registers in CallFrameShuffler isn't supported
- // on 32-bit architectures yet.
+ // ... as well as the callee saved registers
m_lockedRegisters.exclude(RegisterSet::vmCalleeSaveRegisters());
-#endif
ASSERT(!data.callee.isInJSStack() || data.callee.virtualRegister().isLocal());
addNew(CallFrameSlot::callee, data.callee);
@@ -68,17 +63,21 @@
addNew(virtualRegisterForArgumentIncludingThis(i), data.args[i]);
}
-#if USE(JSVALUE64)
for (Reg reg = Reg::first(); reg <= Reg::last(); reg = reg.next()) {
if (!data.registers[reg].isSet())
continue;
- if (reg.isGPR())
+ if (reg.isGPR()) {
+#if USE(JSVALUE64)
addNew(JSValueRegs(reg.gpr()), data.registers[reg]);
- else
+#elif USE(JSVALUE32_64)
+ addNew(reg.gpr(), data.registers[reg]);
+#endif
+ } else
addNew(reg.fpr(), data.registers[reg]);
}
+#if USE(JSVALUE64)
m_numberTagRegister = data.numberTagRegister;
if (m_numberTagRegister != InvalidGPRReg)
lockGPR(m_numberTagRegister);
Modified: trunk/Source/_javascript_Core/jit/CallFrameShuffler.h (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/CallFrameShuffler.h 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/CallFrameShuffler.h 2021-10-27 15:34:42 UTC (rev 284923)
@@ -117,6 +117,15 @@
#if USE(JSVALUE64)
data.registers[reg] = cachedRecovery->recovery();
+#elif USE(JSVALUE32_64)
+ ValueRecovery recovery = cachedRecovery->recovery();
+ if (recovery.technique() == DisplacedInJSStack) {
+ JSValueRegs wantedJSValueReg = cachedRecovery->wantedJSValueRegs();
+ ASSERT(reg == wantedJSValueReg.payloadGPR() || reg == wantedJSValueReg.tagGPR());
+ bool inTag = reg == wantedJSValueReg.tagGPR();
+ data.registers[reg] = ValueRecovery::calleeSaveRegDisplacedInJSStack(recovery.virtualRegister(), inTag);
+ } else
+ data.registers[reg] = recovery;
#else
RELEASE_ASSERT_NOT_REACHED();
#endif
@@ -664,6 +673,32 @@
cachedRecovery->setWantedJSValueRegs(jsValueRegs);
}
+#if USE(JSVALUE32_64)
+ void addNew(GPRReg gpr, ValueRecovery recovery)
+ {
+ ASSERT(gpr != InvalidGPRReg && !m_newRegisters[gpr]);
+ ASSERT(recovery.technique() == Int32DisplacedInJSStack
+ || recovery.technique() == Int32TagDisplacedInJSStack);
+ CachedRecovery* cachedRecovery = addCachedRecovery(recovery);
+ if (JSValueRegs oldRegs { cachedRecovery->wantedJSValueRegs() }) {
+ // Combine with the other CSR in the same virtual register slot
+ ASSERT(oldRegs.tagGPR() == InvalidGPRReg);
+ ASSERT(oldRegs.payloadGPR() != InvalidGPRReg && oldRegs.payloadGPR() != gpr);
+ if (recovery.technique() == Int32DisplacedInJSStack) {
+ ASSERT(cachedRecovery->recovery().technique() == Int32TagDisplacedInJSStack);
+ cachedRecovery->setWantedJSValueRegs(JSValueRegs(oldRegs.payloadGPR(), gpr));
+ } else {
+ ASSERT(cachedRecovery->recovery().technique() == Int32DisplacedInJSStack);
+ cachedRecovery->setWantedJSValueRegs(JSValueRegs(gpr, oldRegs.payloadGPR()));
+ }
+ cachedRecovery->setRecovery(
+ ValueRecovery::displacedInJSStack(recovery.virtualRegister(), DataFormatJS));
+ } else
+ cachedRecovery->setWantedJSValueRegs(JSValueRegs::payloadOnly(gpr));
+ m_newRegisters[gpr] = cachedRecovery;
+ }
+#endif
+
void addNew(FPRReg fpr, ValueRecovery recovery)
{
ASSERT(fpr != InvalidFPRReg && !m_newRegisters[fpr]);
Modified: trunk/Source/_javascript_Core/jit/CallFrameShuffler32_64.cpp (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/CallFrameShuffler32_64.cpp 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/CallFrameShuffler32_64.cpp 2021-10-27 15:34:42 UTC (rev 284923)
@@ -124,8 +124,11 @@
if (resultGPR == InvalidGPRReg || m_registers[resultGPR] || m_lockedRegisters.get(resultGPR))
resultGPR = getFreeGPR();
ASSERT(resultGPR != InvalidGPRReg);
- m_jit.loadPtr(address.withOffset(PayloadOffset), resultGPR);
- updateRecovery(location,
+ if (location.recovery().technique() == Int32TagDisplacedInJSStack)
+ m_jit.loadPtr(address.withOffset(TagOffset), resultGPR);
+ else
+ m_jit.loadPtr(address.withOffset(PayloadOffset), resultGPR);
+ updateRecovery(location,
ValueRecovery::inGPR(resultGPR, location.recovery().dataFormat()));
if (verbose)
dataLog(location.recovery(), "\n");
@@ -190,15 +193,9 @@
if (wantedTagGPR != InvalidGPRReg) {
ASSERT(!m_lockedRegisters.get(wantedTagGPR));
if (CachedRecovery* currentTag { m_registers[wantedTagGPR] }) {
- if (currentTag == &location) {
- if (verbose)
- dataLog(" + ", wantedTagGPR, " is OK\n");
- } else {
- // This can never happen on 32bit platforms since we
- // have at most one wanted JSValueRegs, for the
- // callee, and no callee-save registers.
- RELEASE_ASSERT_NOT_REACHED();
- }
+ RELEASE_ASSERT(currentTag == &location);
+ if (verbose)
+ dataLog(" + ", wantedTagGPR, " is OK\n");
}
}
@@ -205,13 +202,9 @@
if (wantedPayloadGPR != InvalidGPRReg) {
ASSERT(!m_lockedRegisters.get(wantedPayloadGPR));
if (CachedRecovery* currentPayload { m_registers[wantedPayloadGPR] }) {
- if (currentPayload == &location) {
- if (verbose)
- dataLog(" + ", wantedPayloadGPR, " is OK\n");
- } else {
- // See above
- RELEASE_ASSERT_NOT_REACHED();
- }
+ RELEASE_ASSERT(currentPayload == &location);
+ if (verbose)
+ dataLog(" + ", wantedPayloadGPR, " is OK\n");
}
}
Modified: trunk/Source/_javascript_Core/jit/GPRInfo.h (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/GPRInfo.h 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/GPRInfo.h 2021-10-27 15:34:42 UTC (rev 284923)
@@ -553,7 +553,7 @@
class GPRInfo {
public:
typedef GPRReg RegisterType;
- static constexpr unsigned numberOfRegisters = 9;
+ static constexpr unsigned numberOfRegisters = 10;
static constexpr unsigned numberOfArgumentRegisters = NUMBER_OF_ARGUMENT_REGISTERS;
// Temporary registers.
@@ -582,7 +582,7 @@
static GPRReg toRegister(unsigned index)
{
ASSERT(index < numberOfRegisters);
- static const GPRReg registerForIndex[numberOfRegisters] = { regT0, regT1, regT2, regT3, regT4, regT5, regT6, regT7, regCS1 };
+ static const GPRReg registerForIndex[numberOfRegisters] = { regT0, regT1, regT2, regT3, regT4, regT5, regT6, regT7, regCS0, regCS1 };
return registerForIndex[index];
}
@@ -598,7 +598,7 @@
ASSERT(reg != InvalidGPRReg);
ASSERT(static_cast<int>(reg) < 16);
static const unsigned indexForRegister[16] =
- { 0, 1, 2, 3, 7, 6, InvalidIndex, InvalidIndex, 4, 5, 8, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex };
+ { 0, 1, 2, 3, 7, 6, InvalidIndex, InvalidIndex, 4, 5, 9, 8, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex };
unsigned result = indexForRegister[reg];
return result;
}
Modified: trunk/Source/_javascript_Core/jit/RegisterSet.cpp (284922 => 284923)
--- trunk/Source/_javascript_Core/jit/RegisterSet.cpp 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/jit/RegisterSet.cpp 2021-10-27 15:34:42 UTC (rev 284923)
@@ -254,6 +254,7 @@
result.set(GPRInfo::regCS6);
#endif
#elif CPU(ARM_THUMB2)
+ result.set(GPRInfo::regCS0);
result.set(GPRInfo::regCS1);
#elif CPU(ARM64)
static_assert(GPRInfo::regCS8 == GPRInfo::numberTagRegister, "");
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (284922 => 284923)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-10-27 15:34:42 UTC (rev 284923)
@@ -79,8 +79,11 @@
# After calling, calling bytecode is claiming input registers are not used.
macro dispatchAfterCall(size, opcodeStruct, valueProfileName, dstVirtualRegister, dispatch)
loadi ArgumentCountIncludingThis + TagOffset[cfr], PC
- loadp CodeBlock[cfr], PB
- loadp CodeBlock::m_instructionsRawPointer[PB], PB
+ if C_LOOP or C_LOOP_WIN
+ # On non C_LOOP builds, CSR restore takes care of this.
+ loadp CodeBlock[cfr], PB
+ loadp CodeBlock::m_instructionsRawPointer[PB], PB
+ end
get(size, opcodeStruct, dstVirtualRegister, t3)
storei r1, TagOffset[cfr, t3, 8]
storei r0, PayloadOffset[cfr, t3, 8]
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (284922 => 284923)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-10-27 15:29:52 UTC (rev 284922)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-10-27 15:34:42 UTC (rev 284923)
@@ -82,8 +82,11 @@
# After calling, calling bytecode is claiming input registers are not used.
macro dispatchAfterCall(size, opcodeStruct, valueProfileName, dstVirtualRegister, dispatch)
loadPC()
- loadp CodeBlock[cfr], PB
- loadp CodeBlock::m_instructionsRawPointer[PB], PB
+ if C_LOOP or C_LOOP_WIN
+ # On non C_LOOP builds, CSR restore takes care of this.
+ loadp CodeBlock[cfr], PB
+ loadp CodeBlock::m_instructionsRawPointer[PB], PB
+ end
get(size, opcodeStruct, dstVirtualRegister, t1)
storeq r0, [cfr, t1, 8]
metadata(size, opcodeStruct, t2, t1)
