[webkit-reviews] review granted: [Bug 239578] [JSC] Remove TempRegisterSet : [Attachment 458095] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 239578: [JSC] Remove TempRegisterSet
https://bugs.webkit.org/show_bug.cgi?id=239578

Attachment 458095: Patch

https://bugs.webkit.org/attachment.cgi?id=458095&action=review




--- Comment #6 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 458095
  --> https://bugs.webkit.org/attachment.cgi?id=458095
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.