From 8849ac4d01581ed0b4ebc59e0a5cb9fafe0e7a25 Mon Sep 17 00:00:00 2001 From: "Mikhail R. Gadelha" Date: Fri, 14 Nov 2025 09:41:42 -0800 Subject: [PATCH] Don't issue a ldrd when the register holding the address is also a destination https://bugs.webkit.org/show_bug.cgi?id=302469 Reviewed by Justin Michaud. This patch expands a ldrd into a pair of ldr if any of the destination registers is also the register holding the memory address. Before: 0xf1502462: ldrd r0, r1, [r0] After: 0xf1502462: ldr r1, [r0, #4] 0xf1502464: ldr r0, [r0] * Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h: (JSC::MacroAssemblerARMv7::loadPair32): Canonical link: https://commits.webkit.org/303043@main --- .../assembler/MacroAssemblerARMv7.h | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h b/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h index 08b15d9fc4b62..9bb3d5cfb6020 100644 --- a/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h +++ b/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h @@ -940,23 +940,32 @@ class MacroAssemblerARMv7 : public AbstractMacroAssembler { loadPair32(Address(scratch), dest1, dest2); } else { ASSERT(dest1 != dest2); // If it is the same, ldp becomes illegal instruction. - int32_t absOffset = address.u.offset; - if (absOffset < 0) - absOffset = -absOffset; - if (!(absOffset & ~0x3fc)) { - if ((dest1 == addressTempRegister) || (dest2 == addressTempRegister)) - invalidateCachedAddressTempRegister(); - if ((dest1 == dataTempRegister) || (dest2 == dataTempRegister)) - cachedDataTempRegister().invalidate(); - m_assembler.ldrd(dest1, dest2, address.base, address.u.offset, /* index: */ true, /* wback: */ false); - } else if (address.base == dest1) { + // Check if dest1 or dest2 aliases the base register to avoid UNPREDICTABLE ldrd behavior + if (address.base == dest1) { + // Load high word first to avoid clobbering base register ArmAddress highAddress(address.base, address.u.offset + 4); load32(highAddress, dest2); load32(address, dest1); - } else { + } else if (address.base == dest2) { + // Load low word first to avoid clobbering base register load32(address, dest1); ArmAddress highAddress(address.base, address.u.offset + 4); load32(highAddress, dest2); + } else { + int32_t absOffset = address.u.offset; + if (absOffset < 0) + absOffset = -absOffset; + if (!(absOffset & ~0x3fc)) { + if ((dest1 == addressTempRegister) || (dest2 == addressTempRegister)) + invalidateCachedAddressTempRegister(); + if ((dest1 == dataTempRegister) || (dest2 == dataTempRegister)) + cachedDataTempRegister().invalidate(); + m_assembler.ldrd(dest1, dest2, address.base, address.u.offset, /* index: */ true, /* wback: */ false); + } else { + load32(address, dest1); + ArmAddress highAddress(address.base, address.u.offset + 4); + load32(highAddress, dest2); + } } } }