JIT thunks should work on arm64_32
Created attachment 401694 [details] Patch
Created attachment 401752 [details] Patch
Created attachment 401755 [details] Patch
Created attachment 401768 [details] Patch
Comment on attachment 401768 [details] Patch Looks just right to me, but I probably shouldn't review because I’m not familiar enough with this.
Comment on attachment 401768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401768&action=review r=me > Source/JavaScriptCore/ChangeLog:16 > + 2) MacroAssembler::*Ptr functions call 32/64 bit variants based on > + Address space size rather than cpu architecture. What about call sites where they're using Ptr as a shortcut for 64-bit? Will those be vetted and changed later on? > Source/JavaScriptCore/ChangeLog:23 > + 5) numberOfDFGCompiles should report a big number for useBaselineJIT=0 you should say why > Source/JavaScriptCore/assembler/MacroAssembler.h:1253 > + bool shouldBlindDouble(double value) this indentation looks off > Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:155 > + if (b == ARM64Registers::sp) > + std::swap(a, b); is this required by the architecture? > Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:2510 > void signExtend32ToPtr(TrustedImm32 imm, RegisterID dest) > { > - move(TrustedImmPtr(reinterpret_cast<void*>(static_cast<intptr_t>(imm.m_value))), dest); > + move(TrustedImm64(imm.m_value), dest); > } Why is this needed for address32? Shouldn't we doing a 32-bit-mov? > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:447 > + if (!imm.m_value) > + move(src, dest); > + else > + m_assembler.ror(dest, src, imm.m_value & 0x1f); do these have the same semantics for what happen to upper bits?
Comment on attachment 401768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401768&action=review >> Source/JavaScriptCore/ChangeLog:16 >> + Address space size rather than cpu architecture. > > What about call sites where they're using Ptr as a shortcut for 64-bit? Will those be vetted and changed later on? Yeah, I'm going through those as I go. I couldn't think of an automated way of checking for this with the way our current code is laid out. Ideally, we'd have something that returned the sizeof(<member>) in all our offsetOf* functions so we can assert it's the correct size. >> Source/JavaScriptCore/ChangeLog:23 >> + 5) numberOfDFGCompiles should report a big number for useBaselineJIT=0 > > you should say why Sure. >> Source/JavaScriptCore/assembler/MacroAssembler.h:1253 >> + bool shouldBlindDouble(double value) > > this indentation looks off indeed, will fix. >> Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:155 >> + std::swap(a, b); > > is this required by the architecture? Not sure, I just copied the code for add64... I didn't vet it in detail. >> Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:2510 >> } > > Why is this needed for address32? Shouldn't we doing a 32-bit-mov? You sometimes need to signExtend to the full register size as the instruction operates on the full 64-bit operand. This should likely be renamed to signExtendToRegister as that is how we use for the most part but identifying which spots are which is hard. >> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:447 >> + m_assembler.ror(dest, src, imm.m_value & 0x1f); > > do these have the same semantics for what happen to upper bits? Huh? This is only for armv7? What upper bits are you talking about?
Created attachment 401912 [details] Patch for landing
Committed r263049: <https://trac.webkit.org/changeset/263049> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401912 [details].
<rdar://problem/64373326>