[Webkit-unassigned] [Bug 156593] AffectsNextSibling style relation marking is inefficient

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=156593

Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #276426|review?                     |review+
              Flags|                            |

--- Comment #4 from Benjamin Poulain <benjamin at webkit.org> ---
Comment on attachment 276426
  --> https://bugs.webkit.org/attachment.cgi?id=276426
patch

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

> Source/WebCore/css/SelectorChecker.cpp:94
> +        if (last.type == Style::Relation::AffectsNextSibling && last.element->previousElementSibling() == &element) {

It would likely be better to do:
    element ->nextElementSibling() == last.element
to avoid touching the memory of last.element again.

> Source/WebCore/cssjit/SelectorCompiler.cpp:2214
> +        LocalRegister lastRelation(m_registerAllocator);
> +        m_assembler.load32(sizeAddress, lastRelation);

It would be better to have this load above the mergeFailure above.

Currently the CPU will have to do two loads:
1) m_assembler.branchTest32(Assembler::Zero, sizeAddress)
2) m_assembler.load32(sizeAddress, lastRelation);

If you write:
LocalRegister size(m_registerAllocator);
m_assembler.load32(sizeAddress, size);
mergeFailure.append(m_assembler.branchTest32(Assembler::Zero, size));
m_assembler.sub32(Assembler::TrustedImm32(1), size);
...

Then you only need one load.

> Source/WebCore/cssjit/SelectorCompiler.cpp:2237
> +        m_assembler.load32(valueAddress, value);
> +        m_assembler.add32(Assembler::TrustedImm32(1), value);
> +        m_assembler.store32(value, valueAddress);

If you want you can do 
    m_assembler.add32(Assembler::TrustedImm32(1), valueAddress);

That's one instruction on x86. On ARM it generates the same code as you wrote.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160414/4aefaa0b/attachment.html>