Bug 131602 – CSS JIT: compile the :nth-child() pseudo class

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

[Benjamin Poulain]

CSS JIT: compile the :nth-child() pseudo class

[Benjamin Poulain]

Created attachment 229256 [details]
Patch

[Build Bot]

Comment on attachment 229256 [details]
Patch

Attachment 229256 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5264259642556416

New failing tests:
http/tests/security/video-poster-cross-origin-crash.html

[Build Bot]

Created attachment 229257 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Benjamin Poulain]

Created attachment 229258 [details]
Patch

[Andreas Kling]

Comment on attachment 229258 [details]
Patch

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

> Source/WebCore/cssjit/SelectorCompiler.cpp:1025
> +        failureCases.append(m_assembler.branchTest32(Assembler::NonZero, inputDividend, Assembler::TrustedImm32(1)));

Could this be even tighter with branchTest8()?

> Source/WebCore/cssjit/SelectorCompiler.cpp:2016
> +                failureCases.append(m_assembler.branchTest32(Assembler::Zero, elementCounter, Assembler::TrustedImm32(1)));

Could this be even tighter with branchTest8()?

[Andreas Kling]

Comment on attachment 229258 [details]
Patch

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

> Source/WebCore/cssjit/SelectorCompiler.cpp:1025
> +        failureCases.append(m_assembler.branchTest32(Assembler::NonZero, inputDividend, Assembler::TrustedImm32(1)));

Could this be even tighter with branchTest8()?

> Source/WebCore/cssjit/SelectorCompiler.cpp:2016
> +                failureCases.append(m_assembler.branchTest32(Assembler::Zero, elementCounter, Assembler::TrustedImm32(1)));

Could this be even tighter with branchTest8()?

[Andreas Kling]

I wonder if we could shuffle the Node flags so the ones CSS JIT cares about are testable with 8-bit tests. (I also wonder if that would help anything.)

[Benjamin Poulain]

(In reply to comment #7)
> I wonder if we could shuffle the Node flags so the ones CSS JIT cares about are testable with 8-bit tests. (I also wonder if that would help anything.)

If you look at test32, you'll notice that I already implemented that :)

Depending on the mask, it will select 8 bits instructions :)

Also, if you use Address with test32, it will be smart about changing the offset to perform a 8 bits operation :)

[Andreas Kling]

(In reply to comment #8)
> (In reply to comment #7)
> > I wonder if we could shuffle the Node flags so the ones CSS JIT cares about are testable with 8-bit tests. (I also wonder if that would help anything.)
> 
> If you look at test32, you'll notice that I already implemented that :)
> 
> Depending on the mask, it will select 8 bits instructions :)
> 
> Also, if you use Address with test32, it will be smart about changing the offset to perform a 8 bits operation :)

LOL okay! Good job already then :D

[Andreas Kling]

Comment on attachment 229258 [details]
Patch

r=me.

We can definitely improve upon the modulo function if we decide that's useful, but this is totally fine for landing now!

[Benjamin Poulain]

Comment on attachment 229258 [details]
Patch

(In reply to comment #10)
> (From update of attachment 229258 [details])
> r=me.
> 
> We can definitely improve upon the modulo function if we decide that's useful, but this is totally fine for landing now!

Thanks!

[WebKit Commit Bot]

Comment on attachment 229258 [details]
Patch

Clearing flags on attachment: 229258

Committed r167218: <http://trac.webkit.org/changeset/167218>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.