Also consider extracting AssemblyHelpers::emitLoadPrototype() and vet InstanceOfGeneric case of AccessCase::generateWithGuard().
Created attachment 402418 [details] Patch
Created attachment 402422 [details] Patch Attempt to fix 32-bit builds.
Comment on attachment 402422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402422&action=review r=me > Source/JavaScriptCore/bytecode/AccessCase.cpp:1287 > + jit.emitLoadPrototype(vm, valueGPR, scratch2GPR, scratchGPR, failAndIgnore); #if USE(JSVALUE64) JSValueRegs resultRegs(scratch2GPR); #else JSValueRegs resultRegs(scratchGPR, scratch2GPR); #endif jit.emitLoadPrototype(vm, valueGPR, resultRegs, scratchGPR, failAndIgnore); > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13952 > + m_jit.emitLoadPrototype(vm(), objectGPR, tempGPR, temp2GPR, slowCases); m_jit.emitLoadPrototype(vm(), objectGPR, resultRegs, temp2GPR, slowCases); > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13969 > + m_jit.emitLoadPrototype(vm(), valueGPR, tempGPR, temp2GPR, slowCases); m_jit.emitLoadPrototype(vm(), valueGPR, resultRegs, temp2GPR, slowCases); > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4450 > + m_out.appendTo(slowPath, continuation); m_out.appendTo(slowPath, fastPath); > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4470 > + m_out.appendTo(isObjectPath, continuation); m_out.appendTo(isObjectPath, slowPath); > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4478 > + m_out.appendTo(slowPath, continuation); m_out.appendTo(slowPath, fastPath); > Source/JavaScriptCore/jit/AssemblyHelpers.cpp:395 > +void AssemblyHelpers::emitLoadPrototype(VM& vm, RegisterID source, RegisterID dest, RegisterID scratch, JumpList& slowPath) Let's accept `GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR` instead. Changing the meaning of scratch to part of resultRegs inside function looks weird. We should get JSValueRegs resultRegs in this function's parameter. And let's note that scratch can be a resultRegs.tagGPR(). You can insert, ASSERT(resultRegs.payloadGPR() != objectGPR); ASSERT(resultRegs.payloadGPR() != scratchGPR); ASSERT(objectGPR != scratchGPR); > Source/JavaScriptCore/jit/AssemblyHelpers.cpp:397 > + emitLoadStructure(vm, source, dest, scratch); emitLoadStructure(vm, objectGPR, resultRegs.payloadGPR(), scratchGPR); > Source/JavaScriptCore/jit/AssemblyHelpers.cpp:408 > +#if USE(JSVALUE64) > + JSValueRegs resultRegs(dest); > +#else > + JSValueRegs resultRegs(scratch, dest); > +#endif This is not necessary. > Source/JavaScriptCore/jit/AssemblyHelpers.cpp:410 > + loadValue(MacroAssembler::Address(dest, Structure::prototypeOffset()), resultRegs); loadValue(MacroAssembler::Address(resultRegs.payloadGPR(), Structure::prototypeOffset()), resultRegs); > Source/JavaScriptCore/jit/AssemblyHelpers.cpp:412 > + loadValue(MacroAssembler::Address(source, offsetRelativeToBase(knownPolyProtoOffset)), resultRegs); loadValue(MacroAssembler::Address(objectGPR, offsetRelativeToBase(knownPolyProtoOffset)), resultRegs); > Source/JavaScriptCore/jit/JITOpcodes.cpp:1753 > + JSValueRegs resultRegs(regT2); Add GPRReg scratchGPR = regT3; > Source/JavaScriptCore/jit/JITOpcodes.cpp:1756 > + JSValueRegs resultRegs(regT3, regT2); Add GPRReg scratchGPR = regT1; ASSERT(valueRegs.tagGPR() == scratchGPR); > Source/JavaScriptCore/jit/JITOpcodes.cpp:1762 > + slowCases.append(branchIfNotObject(regT0)); Let's use valueRegs.payloadGPR(). > Source/JavaScriptCore/jit/JITOpcodes.cpp:1764 > + emitLoadPrototype(vm(), regT0, regT2, regT3, slowCases); Let's use it to something like this. ` emitLoadPrototype(vm(), valueRegs.payloadGPR(), resultRegs, scratchGPR, slowCases); `
Created attachment 402651 [details] Patch Accept JSValueRegs in emitLoadPrototype, add ASSERTs, fix FTL blocks linking.
Committed r263470: <https://trac.webkit.org/changeset/263470> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402651 [details].
<rdar://problem/64716119>
Comment on attachment 402651 [details] Patch lgtm too