[JSC] Get 32-bit ports back into building order
Created attachment 381787 [details] Patch
Created attachment 381788 [details] Patch for landing
Comment on attachment 381788 [details] Patch for landing Clearing flags on attachment: 381788 Committed r251534: <https://trac.webkit.org/changeset/251534>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56572554>
Comment on attachment 381788 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=381788&action=review LGTM > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1891 > +#if USE(JSVALUE64) Good catch. I didn't see this yesterday.
Comment on attachment 381788 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=381788&action=review > Source/JavaScriptCore/dfg/DFGOSRExit.cpp:828 > + uint32_t locationBits = CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(instruction))).bits(); This seems wrong... my change made CallSiteIndex always use the bytecode index rather than the instruction pointer on 32-bit. I think you want: uint32_t locationBits = CallSiteIndex(BytecodeIndex(codeOrigin->bytecodeIndex())).bits(); > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:332 > + uint32_t locationBits = CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(instruction))).bits(); ditto. > Source/JavaScriptCore/interpreter/CallFrame.cpp:119 > + CallSiteIndex callSite(BytecodeIndex(bitwise_cast<uint32_t>(vpc))); ditto. > Source/JavaScriptCore/jit/JITCall32_64.cpp:291 > + uint32_t locationBits = CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(instruction))).bits(); ditto. > Source/JavaScriptCore/jit/JITInlines.h:131 > +#if USE(JSVALUE32_64) > + const Instruction* instruction = m_codeBlock->instructions().at(m_bytecodeIndex.offset()).ptr(); > + uint32_t locationBits = CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(instruction))).bits(); > +#else I think this is wrong. > Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:1389 > + logShadowChickenTailPacket(shadowPacketReg, thisRegs, regT3, m_codeBlock, CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(currentInstruction)))); ditto. > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:210 > + m_codeBlock, CodeOrigin(m_bytecodeIndex), CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(currentInstruction))), RegisterSet::stubUnavailableRegisters(), ditto. > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:440 > + m_codeBlock, CodeOrigin(m_bytecodeIndex), CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(currentInstruction))), RegisterSet::stubUnavailableRegisters(), ditto. > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:504 > + m_codeBlock, CodeOrigin(m_bytecodeIndex), CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(currentInstruction))), RegisterSet::stubUnavailableRegisters(), ditto. > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:543 > + m_codeBlock, CodeOrigin(m_bytecodeIndex), CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(currentInstruction))), RegisterSet::stubUnavailableRegisters(), ditto.
(In reply to Keith Miller from comment #7) > Comment on attachment 381788 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=381788&action=review > > > Source/JavaScriptCore/dfg/DFGOSRExit.cpp:828 > > + uint32_t locationBits = CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(instruction))).bits(); > > This seems wrong... my change made CallSiteIndex always use the bytecode > index rather than the instruction pointer on 32-bit. > > I think you want: > uint32_t locationBits = > CallSiteIndex(BytecodeIndex(codeOrigin->bytecodeIndex())).bits(); > Besides the building errors, these changes were additionally causing plenty of test errors. Problem is that on 32-bit platforms we don't yet have a PC base register that would hold the base address of the instructions data. Ideally we would fish out a register we could use for this, and eliminate the different approach here. I opened bug #203563 to cover this.
Comment on attachment 381788 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=381788&action=review >>> Source/JavaScriptCore/dfg/DFGOSRExit.cpp:828 >>> + uint32_t locationBits = CallSiteIndex(BytecodeIndex(bitwise_cast<uint32_t>(instruction))).bits(); >> >> This seems wrong... my change made CallSiteIndex always use the bytecode index rather than the instruction pointer on 32-bit. >> >> I think you want: >> uint32_t locationBits = CallSiteIndex(BytecodeIndex(codeOrigin->bytecodeIndex())).bits(); > > Besides the building errors, these changes were additionally causing plenty of test errors. > > Problem is that on 32-bit platforms we don't yet have a PC base register that would hold the base address of the instructions data. Ideally we would fish out a register we could use for this, and eliminate the different approach here. > > I opened bug #203563 to cover this. Keith, are you sure you made that change? ToT says: const Instruction* CallFrame::currentVPC() const { return bitwise_cast<Instruction*>(callSiteIndex().bits()); }