[webkit-reviews] review granted: [Bug 133834] make RegisterAllocator smarter : [Attachment 233013] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 133834: make RegisterAllocator smarter
https://bugs.webkit.org/show_bug.cgi?id=133834

Attachment 233013: Patch
https://bugs.webkit.org/attachment.cgi?id=233013&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=233013&action=review


Very neat! I love it.

> Source/WebCore/cssjit/RegisterAllocator.h:93
> +	   RELEASE_ASSERT(m_registers.size());

I think you should put this assertion one line above. Otherwise it looks a
little weird accessing the first element before checking if the size is valid.

> Source/WebCore/cssjit/RegisterAllocator.h:113
> +    JSC::MacroAssembler::RegisterID
allocateRegisterWithPreference(JSC::MacroAssembler::RegisterID registerID)

registerID -> preferedRegisterID?

> Source/WebCore/cssjit/RegisterAllocator.h:115
> +	   UNUSED_PARAM(registerID);

Forgot this?

> Source/WebCore/cssjit/RegisterAllocator.h:133
> +	   for (auto reg : m_registers)

reg -> register