Modified: trunk/Source/_javascript_Core/ChangeLog (291338 => 291339)
--- trunk/Source/_javascript_Core/ChangeLog 2022-03-16 10:19:42 UTC (rev 291338)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-03-16 12:05:08 UTC (rev 291339)
@@ -1,3 +1,43 @@
+2022-03-16 Angelos Oikonomopoulos <ange...@igalia.com>
+
+ MacroAssemblerARMv7: Be friendlier to DisallowMacroScratchRegisterUsage
+ https://bugs.webkit.org/show_bug.cgi?id=237888
+
+ Reviewed by Žan Doberšek.
+
+ Only check that we're allowed to use the scratch register at sites
+ where we're using it implicitly. When it's explicitly passed in by the
+ caller, use invalidateCachedAddressTempRegister to invalidate it
+ without asserting anything about m_allowScratchRegister.
+
+ Since helpers can explictly make use of addressTempRegister, an
+ argument can be made that this is still fragile (i.e. future changes
+ could run into this). The alternative would be to have the topmost caller
+ do fine-grained management of DisallowMacroScratchRegisterUsage,
+ allowing it around explicit calls to MacroAssemblerARMv7 with
+ scratchRegister() in the arguments and disallowing it for helpers.
+
+ As there are currently no helpers that would trip this, this patch opts
+ for the former approach, to make DisallowMacroScratchRegisterUsage
+ easier to work with (there'll be more usage of the API in an upcoming
+ wasm32 patch).
+
+ * assembler/MacroAssemblerARMv7.h:
+ (JSC::MacroAssemblerARMv7::scratchRegister):
+ (JSC::MacroAssemblerARMv7::load32):
+ (JSC::MacroAssemblerARMv7::load16):
+ (JSC::MacroAssemblerARMv7::load16SignedExtendTo32):
+ (JSC::MacroAssemblerARMv7::load8):
+ (JSC::MacroAssemblerARMv7::load8SignedExtendTo32):
+ (JSC::MacroAssemblerARMv7::loadPair32):
+ (JSC::MacroAssemblerARMv7::move):
+ (JSC::MacroAssemblerARMv7::farJump):
+ (JSC::MacroAssemblerARMv7::setupArmAddress):
+ (JSC::MacroAssemblerARMv7::invalidateCachedAddressTempRegister):
+ * bytecode/CallLinkInfo.cpp:
+ (JSC::CallLinkInfo::emitFastPathImpl):
+ * jit/BaselineJITRegisters.h:
+
2022-03-15 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Add UnlinkedDFG compilation mode enum
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h (291338 => 291339)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2022-03-16 10:19:42 UTC (rev 291338)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2022-03-16 12:05:08 UTC (rev 291339)
@@ -78,7 +78,12 @@
static constexpr unsigned numGPRs = std::initializer_list<int>({ FOR_EACH_GP_REGISTER(DUMMY_REGISTER_VALUE) }).size();
static constexpr unsigned numFPRs = std::initializer_list<int>({ FOR_EACH_FP_REGISTER(DUMMY_REGISTER_VALUE) }).size();
#undef DUMMY_REGISTER_VALUE
- static constexpr RegisterID scratchRegister() { return addressTempRegister; }
+ static constexpr RegisterID s_scratchRegister = addressTempRegister;
+ RegisterID scratchRegister()
+ {
+ RELEASE_ASSERT(m_allowScratchRegister);
+ return s_scratchRegister;
+ }
MacroAssemblerARMv7()
: m_makeJumpPatchable(false)
@@ -679,7 +684,7 @@
void load32(ArmAddress address, RegisterID dest)
{
if (dest == addressTempRegister)
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
else if (dest == dataTempRegister)
cachedDataTempRegister().invalidate();
@@ -698,7 +703,7 @@
void load16(ArmAddress address, RegisterID dest)
{
if (dest == addressTempRegister)
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
else if (dest == dataTempRegister)
cachedDataTempRegister().invalidate();
@@ -718,7 +723,7 @@
{
ASSERT(address.type == ArmAddress::HasIndex);
if (dest == addressTempRegister)
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
else if (dest == dataTempRegister)
cachedDataTempRegister().invalidate();
@@ -728,7 +733,7 @@
void load8(ArmAddress address, RegisterID dest)
{
if (dest == addressTempRegister)
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
else if (dest == dataTempRegister)
cachedDataTempRegister().invalidate();
@@ -748,7 +753,7 @@
{
ASSERT(address.type == ArmAddress::HasIndex);
if (dest == addressTempRegister)
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
else if (dest == dataTempRegister)
cachedDataTempRegister().invalidate();
@@ -933,7 +938,7 @@
absOffset = -absOffset;
if (!(absOffset & ~0x3fc)) {
if ((dest1 == addressTempRegister) || (dest2 == addressTempRegister))
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
if ((dest1 == dataTempRegister) || (dest2 == dataTempRegister))
cachedDataTempRegister().invalidate();
m_assembler.ldrd(dest1, dest2, address.base, address.u.offset, /* index: */ true, /* wback: */ false);
@@ -1665,7 +1670,7 @@
if (dest == dataTempRegister)
cachedDataTempRegister().invalidate();
else if (dest == addressTempRegister)
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
}
void move(TrustedImmPtr imm, RegisterID dest)
@@ -2054,7 +2059,7 @@
void farJump(RegisterID target, PtrTag)
{
cachedDataTempRegister().invalidate();
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
m_assembler.bx(target);
}
@@ -2062,7 +2067,7 @@
{
move(target, addressTempRegister);
cachedDataTempRegister().invalidate();
- cachedAddressTempRegister().invalidate();
+ invalidateCachedAddressTempRegister();
m_assembler.bx(addressTempRegister);
}
@@ -2529,8 +2534,8 @@
m_assembler.add(scratch, address.base, imm);
} else {
move(TrustedImm32(address.offset), addressTempRegister);
+ m_assembler.add(addressTempRegister, addressTempRegister, address.base);
cachedAddressTempRegister().invalidate();
- m_assembler.add(addressTempRegister, addressTempRegister, address.base);
}
return ArmAddress(addressTempRegister, address.index, address.scale);
@@ -2620,6 +2625,14 @@
return m_cachedDataTempRegister;
}
+ ALWAYS_INLINE void invalidateCachedAddressTempRegister()
+ {
+ // This function is intended for when we are explicitly using
+ // addressTempRegister (because the caller supplied it), so it can
+ // ignore m_allowScratchRegister.
+ m_cachedAddressTempRegister.invalidate();
+ }
+
ALWAYS_INLINE CachedTempRegister& cachedAddressTempRegister()
{
RELEASE_ASSERT(m_allowScratchRegister);
Modified: trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp (291338 => 291339)
--- trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2022-03-16 10:19:42 UTC (rev 291338)
+++ trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2022-03-16 12:05:08 UTC (rev 291339)
@@ -354,8 +354,8 @@
slowPath.append(jit.branchPtr(CCallHelpers::NotEqual, calleeAddress, calleeGPR));
} else {
GPRReg scratchGPR = jit.scratchRegister();
+ DisallowMacroScratchRegisterUsage disallowScratch(jit);
jit.loadPtr(CCallHelpers::Address(callLinkInfoGPR, offsetOfCallee()), scratchGPR);
- DisallowMacroScratchRegisterUsage disallowScratch(jit);
goPolymorphic = jit.branchTestPtr(CCallHelpers::NonZero, scratchGPR, CCallHelpers::TrustedImm32(polymorphicCalleeMask));
slowPath.append(jit.branchPtr(CCallHelpers::NotEqual, scratchGPR, calleeGPR));
}
Modified: trunk/Source/_javascript_Core/jit/BaselineJITRegisters.h (291338 => 291339)
--- trunk/Source/_javascript_Core/jit/BaselineJITRegisters.h 2022-03-16 10:19:42 UTC (rev 291338)
+++ trunk/Source/_javascript_Core/jit/BaselineJITRegisters.h 2022-03-16 12:05:08 UTC (rev 291339)
@@ -310,7 +310,7 @@
GPRInfo::regT5
#elif CPU(ARM_THUMB2)
// We are a bit short on registers on ARM_THUMB2, but we can just about get away with this
- MacroAssemblerARMv7::scratchRegister()
+ MacroAssemblerARMv7::s_scratchRegister
#else // Other JSVALUE32_64
GPRInfo::regT8
#endif
