[JSC][32bit] Fix wrong branchAdd32 assembly for ARMv7
Created attachment 440502 [details] Patch
Comment on attachment 440502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440502&action=review > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:1859 > + // Do the add. > + ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(imm.m_value); > + if (armImm.isValid()) > + m_assembler.add_S(dataTempRegister, dataTempRegister, armImm); > + else { > + move(imm, addressTempRegister); > + m_assembler.add_S(dataTempRegister, dataTempRegister, addressTempRegister); > + } Can we add an add32 with flags helper that is this? It seems like that could be useful elsewhere in this file.
Created attachment 440530 [details] Patch
Comment on attachment 440530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440530&action=review r=me with changes. > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:200 > + void add32(TrustedImm32 imm, Address address, bool updateFlags = false) Let's define it as add32Impl helper private function, and define add32 as void add32(TrustedImm32 imm, Address address) { constexpr bool updateFlags = false; add32Impl(imm, address, updateFlags); } > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:229 > + void add32(TrustedImm32 imm, AbsoluteAddress address, bool updateFlags = false) Let's define it as add32Impl helper private function, and define add32 as void add32(TrustedImm32 imm, AbsoluteAddress address) { constexpr bool updateFlags = false; add32Impl(imm, address, updateFlags); } > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:1861 > + add32(imm, dest, true); Let's make it readable by, constexpr bool updateFlags = true; add32(imm, dest, updateFlags); > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:1867 > + add32(imm, dest, true); Let's make it readable by, constexpr bool updateFlags = true; add32(imm, dest, updateFlags);
Created attachment 441077 [details] Patch
Committed r284103 (242931@main): <https://commits.webkit.org/242931@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441077 [details].
<rdar://problem/84206141>