| Summary: | CSS JIT: compile the :nth-child() pseudo class | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||||
| Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | allan.jensen, buildbot, commit-queue, eric.carlson, esprehn+autocc, glenn, gyuyoung.kim, jer.noble, kangil.han, kling, kondapallykalyan, macpherson, menard, philipj, rniwa, sergio | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Benjamin Poulain
2014-04-13 19:12:38 PDT
Created attachment 229256 [details]
Patch
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 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
Created attachment 229258 [details]
Patch
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()? 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()? 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.) (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 :) (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 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!
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! Comment on attachment 229258 [details] Patch Clearing flags on attachment: 229258 Committed r167218: <http://trac.webkit.org/changeset/167218> All reviewed patches have been landed. Closing bug. |