This way the CallSiteIndex offset values could be used the same way as on 64-bit platforms. More about this in bug #203358.
Created attachment 387651 [details] Patch
Comment on attachment 387651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387651&action=review Early comment. > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:299 > + const PB = csr1 You need to add this register to sets of RegisterSet.cpp. Let's read the comment in RegisterSet.h. 51 JS_EXPORT_PRIVATE static RegisterSet stackRegisters(); 52 JS_EXPORT_PRIVATE static RegisterSet reservedHardwareRegisters(); 53 static RegisterSet runtimeTagRegisters(); 54 static RegisterSet specialRegisters(); // The union of stack, reserved hardware, and runtime registers. 55 JS_EXPORT_PRIVATE static RegisterSet calleeSaveRegisters(); 56 static RegisterSet vmCalleeSaveRegisters(); // Callee save registers that might be saved and used by any tier. 57 static RegisterAtOffsetList* vmCalleeSaveRegisterOffsets(); 58 static RegisterSet llintBaselineCalleeSaveRegisters(); // Registers saved and used by the LLInt. 59 static RegisterSet dfgCalleeSaveRegisters(); // Registers saved and used by the DFG JIT. 60 static RegisterSet ftlCalleeSaveRegisters(); // Registers that might be saved and used by the FTL JIT. 61 #if ENABLE(WEBASSEMBLY) 62 static RegisterSet webAssemblyCalleeSaveRegisters(); // Registers saved and used by the WebAssembly JIT. 63 #endif 64 static RegisterSet volatileRegistersForJSCall(); 65 static RegisterSet stubUnavailableRegisters(); // The union of callee saves and special registers. 66 JS_EXPORT_PRIVATE static RegisterSet macroScratchRegisters(); 67 JS_EXPORT_PRIVATE static RegisterSet allGPRs(); 68 JS_EXPORT_PRIVATE static RegisterSet allFPRs(); 69 static RegisterSet allRegisters(); 70 JS_EXPORT_PRIVATE static RegisterSet argumentGPRS(); 71 72 static RegisterSet registersToNotSaveForJSCall(); 73 static RegisterSet registersToNotSaveForCCall(); We need to add this register to appropriate sets. > Source/JavaScriptCore/offlineasm/arm.rb:73 > ARM_EXTRA_FPRS = [SpecialRegister.new("d7")] Let's update the comments in arm.rb about csr. > Source/JavaScriptCore/offlineasm/arm.rb:105 > + "r3" Looks correct. Keep in sync with GPRInfo.h > Source/JavaScriptCore/offlineasm/arm.rb:115 > + when "csr1" > + "r10" GPRInfo.h and RegisterSet.cpp need to be updated. > Source/JavaScriptCore/offlineasm/mips.rb:138 > "$fp" Let's update the comment in mips.rb. And keep in sync with GPRInfo.h. And add registers to appropriate set in RegisterSet.cpp.
When modifying ARMv7 register definition, keep in mind that ARM registers usage are slightly different in iOS ARM and Linux ARM. See ARMv7Registers.h.
Comment on attachment 387651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387651&action=review > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:912 > + loadp 4[temp], csr1 This means you need to update GPRInfo.h's NUMBER_OF_CALLEE_SAVES_REGISTERS too.
Created attachment 387777 [details] Patch
Created attachment 387779 [details] WIP - Patch
@Yusuke, Thank you very much for the comments.
Created attachment 387811 [details] Patch
Comment on attachment 387651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387651&action=review >> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:299 >> + const PB = csr1 > > You need to add this register to sets of RegisterSet.cpp. > Let's read the comment in RegisterSet.h. > > 51 JS_EXPORT_PRIVATE static RegisterSet stackRegisters(); > 52 JS_EXPORT_PRIVATE static RegisterSet reservedHardwareRegisters(); > 53 static RegisterSet runtimeTagRegisters(); > 54 static RegisterSet specialRegisters(); // The union of stack, reserved hardware, and runtime registers. > 55 JS_EXPORT_PRIVATE static RegisterSet calleeSaveRegisters(); > 56 static RegisterSet vmCalleeSaveRegisters(); // Callee save registers that might be saved and used by any tier. > 57 static RegisterAtOffsetList* vmCalleeSaveRegisterOffsets(); > 58 static RegisterSet llintBaselineCalleeSaveRegisters(); // Registers saved and used by the LLInt. > 59 static RegisterSet dfgCalleeSaveRegisters(); // Registers saved and used by the DFG JIT. > 60 static RegisterSet ftlCalleeSaveRegisters(); // Registers that might be saved and used by the FTL JIT. > 61 #if ENABLE(WEBASSEMBLY) > 62 static RegisterSet webAssemblyCalleeSaveRegisters(); // Registers saved and used by the WebAssembly JIT. > 63 #endif > 64 static RegisterSet volatileRegistersForJSCall(); > 65 static RegisterSet stubUnavailableRegisters(); // The union of callee saves and special registers. > 66 JS_EXPORT_PRIVATE static RegisterSet macroScratchRegisters(); > 67 JS_EXPORT_PRIVATE static RegisterSet allGPRs(); > 68 JS_EXPORT_PRIVATE static RegisterSet allFPRs(); > 69 static RegisterSet allRegisters(); > 70 JS_EXPORT_PRIVATE static RegisterSet argumentGPRS(); > 71 > 72 static RegisterSet registersToNotSaveForJSCall(); > 73 static RegisterSet registersToNotSaveForCCall(); > > We need to add this register to appropriate sets. Done. >> Source/JavaScriptCore/offlineasm/arm.rb:73 >> ARM_EXTRA_FPRS = [SpecialRegister.new("d7")] > > Let's update the comments in arm.rb about csr. Done. >> Source/JavaScriptCore/offlineasm/arm.rb:105 >> + "r3" > > Looks correct. Keep in sync with GPRInfo.h Using `r4` here was divergent from `GPRInfo.h`. this change makes them aligned again. >> Source/JavaScriptCore/offlineasm/arm.rb:115 >> + "r10" > > GPRInfo.h and RegisterSet.cpp need to be updated. Done. >> Source/JavaScriptCore/offlineasm/mips.rb:138 >> "$fp" > > Let's update the comment in mips.rb. And keep in sync with GPRInfo.h. And add registers to appropriate set in RegisterSet.cpp. Done.
Created attachment 387828 [details] Patch
Created attachment 387844 [details] Patch
Comment on attachment 387844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387844&action=review r=me with some comments. > Source/JavaScriptCore/assembler/MIPSRegisters.h:53 > + macro(r17, "s1", 0, 1) \ Was this just a bug in the MIPS calling convention? > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:66 > macro nextInstruction() > - loadb [PC], t0 > + loadb [PB, PC, 1], t0 > leap _g_opcodeMap, t1 > jmp [t1, t0, 4], BytecodePtrTag > end > > macro nextInstructionWide16() > - loadb OpcodeIDNarrowSize[PC], t0 > + loadb OpcodeIDNarrowSize[PB, PC, 1], t0 > leap _g_opcodeMapWide16, t1 > jmp [t1, t0, 4], BytecodePtrTag > end > > macro nextInstructionWide32() > - loadb OpcodeIDNarrowSize[PC], t0 > + loadb OpcodeIDNarrowSize[PB, PC, 1], t0 > leap _g_opcodeMapWide32, t1 > jmp [t1, t0, 4], BytecodePtrTag > end > > macro getuOperandNarrow(opcodeStruct, fieldName, dst) > - loadb constexpr %opcodeStruct%_%fieldName%_index + OpcodeIDNarrowSize[PC], dst > + loadb constexpr %opcodeStruct%_%fieldName%_index + OpcodeIDNarrowSize[PB, PC, 1], dst > end > > macro getOperandNarrow(opcodeStruct, fieldName, dst) > - loadbsi constexpr %opcodeStruct%_%fieldName%_index + OpcodeIDNarrowSize[PC], dst > + loadbsi constexpr %opcodeStruct%_%fieldName%_index + OpcodeIDNarrowSize[PB, PC, 1], dst > end > > macro getuOperandWide16(opcodeStruct, fieldName, dst) > - loadh constexpr %opcodeStruct%_%fieldName%_index * 2 + OpcodeIDWide16Size[PC], dst > + loadh constexpr %opcodeStruct%_%fieldName%_index * 2 + OpcodeIDWide16Size[PB, PC, 1], dst > end > > macro getOperandWide16(opcodeStruct, fieldName, dst) > - loadhsi constexpr %opcodeStruct%_%fieldName%_index * 2 + OpcodeIDWide16Size[PC], dst > + loadhsi constexpr %opcodeStruct%_%fieldName%_index * 2 + OpcodeIDWide16Size[PB, PC, 1], dst > end > > macro getuOperandWide32(opcodeStruct, fieldName, dst) > - loadi constexpr %opcodeStruct%_%fieldName%_index * 4 + OpcodeIDWide32Size[PC], dst > + loadi constexpr %opcodeStruct%_%fieldName%_index * 4 + OpcodeIDWide32Size[PB, PC, 1], dst > end > > macro getOperandWide32(opcodeStruct, fieldName, dst) > - loadis constexpr %opcodeStruct%_%fieldName%_index * 4 + OpcodeIDWide32Size[PC], dst > + loadis constexpr %opcodeStruct%_%fieldName%_index * 4 + OpcodeIDWide32Size[PB, PC, 1], dst > end Can you add a fix me to merge this with the ones in LowLevelInterpreter64.asm? Seems like you can't right now because we don't have a loadhsp (although, I think loadhsi is probably fine).
This also fixes the crash I was seeing on our watch builds. So I think we should land this while undoing my revert.
Created attachment 387905 [details] Patch
Comment on attachment 387844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387844&action=review Thank you very much for the review. >> Source/JavaScriptCore/assembler/MIPSRegisters.h:53 >> + macro(r17, "s1", 0, 1) \ > > Was this just a bug in the MIPS calling convention? I think it would be a problem if we were using such register at some point, but it seems we don't use `$sx` registers besides `s0` (and now `s1`). >> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:66 >> end > > Can you add a fix me to merge this with the ones in LowLevelInterpreter64.asm? Seems like you can't right now because we don't have a loadhsp (although, I think loadhsi is probably fine). Done.
Comment on attachment 387844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387844&action=review >>> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:66 >>> end >> >> Can you add a fix me to merge this with the ones in LowLevelInterpreter64.asm? Seems like you can't right now because we don't have a loadhsp (although, I think loadhsi is probably fine). > > Done. I've refactored `nextInstruction` part out to LowLevelInterpreter.asm, since 64 and 32-bits are the same.
Comment on attachment 387905 [details] Patch Clearing flags on attachment: 387905 Committed r254674: <https://trac.webkit.org/changeset/254674>
All reviewed patches have been landed. Closing bug.
<rdar://problem/58641478>