[JSC] Remove TempRegisterSet
Pull request: https://github.com/WebKit/WebKit/pull/337
Committed r293146 (249842@main): <https://commits.webkit.org/249842@main> Reviewed commits have been landed. Closing PR #337 and removing active labels.
<rdar://problem/92073330>
Re-opened since this is blocked by bug 239613
Created attachment 458095 [details] Patch
Comment on attachment 458095 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458095&action=review r=me > Source/JavaScriptCore/ChangeLog:10 > + We can always use RegisterSet. TempRegisterSet can save several bytes, but we have no code using TempRegisterSet in > + heap-allocated class, so this does not make sense anymore. Instead of TempRegisterSet, we should consistently use > + ScratchRegisterAllocator. I suggest slightly rephrasing this as follows: We can always use RegisterSet. TempRegisterSet can save several bytes, but we have no code using TempRegisterSet in heap-allocated classes. So, this does not make sense anymore. Instead of TempRegisterSet, we will consistently use RegisterSet to pass register info and ScratchRegisterAllocator to manage allocation of temp / scratch registers. > Source/JavaScriptCore/ChangeLog:14 > + We also remove copyCalleeSavesToEntryFrameCalleeSavesBuffer function with no scratch register. We were using TempRegisterSet > + to allocate scratch register, but the caller of this function has particular assumption on how register is allocated from > + TempRegisterSet. This is very fragile and dangerous. We should pass scratch register explicitly in that case. We also remove the copyCalleeSavesToEntryFrameCalleeSavesBuffer function which takes no scratch register. It was using TempRegisterSet to allocate a scratch register, but the caller of this function was making assumptions on how TempRegisterSet will allocate that scratch. This is very fragile and dangerous. We should explicitly pass a scratch register instead in that case.
Comment on attachment 458095 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458095&action=review Thank you! >> Source/JavaScriptCore/ChangeLog:10 >> + ScratchRegisterAllocator. > > I suggest slightly rephrasing this as follows: > > We can always use RegisterSet. TempRegisterSet can save several bytes, but we have no code using TempRegisterSet in > heap-allocated classes. So, this does not make sense anymore. Instead of TempRegisterSet, we will consistently use > RegisterSet to pass register info and ScratchRegisterAllocator to manage allocation of temp / scratch registers. Sounds good, changed. >> Source/JavaScriptCore/ChangeLog:14 >> + TempRegisterSet. This is very fragile and dangerous. We should pass scratch register explicitly in that case. > > We also remove the copyCalleeSavesToEntryFrameCalleeSavesBuffer function which takes no scratch register. It was > using TempRegisterSet to allocate a scratch register, but the caller of this function was making assumptions on how > TempRegisterSet will allocate that scratch. This is very fragile and dangerous. We should explicitly pass a scratch > register instead in that case. Changed.
Committed r293203 (?): <https://commits.webkit.org/r293203>