Bug 239578 – [JSC] Remove TempRegisterSet

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Remove TempRegisterSet

[Yusuke Suzuki]

Pull request: https://github.com/WebKit/WebKit/pull/337

[EWS]

Committed r293146 (249842@main): <https://commits.webkit.org/249842@main>

Reviewed commits have been landed. Closing PR #337 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/92073330>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 239613

[Yusuke Suzuki]

Created attachment 458095 [details]
Patch

[Mark Lam]

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.

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r293203 (?): <https://commits.webkit.org/r293203>