Bug 133506

Summary: CSS JIT: Clean up StackAllocator
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: CSSAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Yusuke Suzuki 2014-06-04 02:03:47 PDT 
Cleaning up CSS JIT SatkcAcllocator code.
Comment 1 Yusuke Suzuki 2014-06-04 02:06:19 PDT 
Created attachment 232469 [details]
Patch
Comment 2 Benjamin Poulain 2014-06-04 16:51:23 PDT 
Comment on attachment 232469 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232469&action=review

Looks good to me.

> Source/WebCore/cssjit/StackAllocator.h:85
>          if (registerCount % 2) {
> -            m_assembler.pushToSave(registerIDs[registerCount - 1]);
> -            m_offsetFromTop += stackUnitInBytes;
> -            stackReferences.append(StackReference(m_offsetFromTop));
> +            stackReferences.append(push(registerIDs[registerCount - 1]));

WebKit coding style: the brackets should go away when the block only has one line.

> Source/WebCore/cssjit/StackAllocator.h:90
>          for (auto registerID : registerIDs) {
> -            m_assembler.pushToSave(registerID);
> -            m_offsetFromTop += stackUnitInBytes;
> -            stackReferences.append(StackReference(m_offsetFromTop));
> +            stackReferences.append(push(registerID));
>          }

Ditto.

> Source/WebCore/cssjit/StackAllocator.h:114
>          if (registerCountOdd) {
> -            RELEASE_ASSERT(stackReferences[registerCount - 1] == m_offsetFromTop);
> -            RELEASE_ASSERT(m_offsetFromTop >= stackUnitInBytes);
> -            m_offsetFromTop -= stackUnitInBytes;
> -            m_assembler.popToRestore(registerIDs[registerCount - 1]);
> +            pop(stackReferences[registerCount - 1], registerIDs[registerCount - 1]);
>          }

Ditto.

> Source/WebCore/cssjit/StackAllocator.h:126
>          for (unsigned i = registerCount; i > 0; --i) {
> -            RELEASE_ASSERT(stackReferences[i - 1] == m_offsetFromTop);
> -            RELEASE_ASSERT(m_offsetFromTop >= stackUnitInBytes);
> -            m_offsetFromTop -= stackUnitInBytes;
> -            m_assembler.popToRestore(registerIDs[i - 1]);
> +            pop(stackReferences[i - 1], registerIDs[i - 1]);
>          }

Ditto.
Comment 3 Yusuke Suzuki 2014-06-05 08:34:38 PDT 
Created attachment 232551 [details]
Patch
Comment 4 Yusuke Suzuki 2014-06-05 08:34:58 PDT 
Comment on attachment 232469 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232469&action=review

Thank you for your review.

>> Source/WebCore/cssjit/StackAllocator.h:85
>> +            stackReferences.append(push(registerIDs[registerCount - 1]));
> 
> WebKit coding style: the brackets should go away when the block only has one line.

Oh! I've missed it. Thank you.
Comment 5 Benjamin Poulain 2014-06-05 11:10:36 PDT 
Comment on attachment 232551 [details]
Patch

Looks good to me.
Comment 6 WebKit Commit Bot 2014-06-05 11:40:54 PDT 
Comment on attachment 232551 [details]
Patch

Clearing flags on attachment: 232551

Committed r169621: <http://trac.webkit.org/changeset/169621>
Comment 7 WebKit Commit Bot 2014-06-05 11:40:56 PDT 
All reviewed patches have been landed.  Closing bug.