<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:benjamin@webkit.org" title="Benjamin Poulain <benjamin@webkit.org>"> <span class="fn">Benjamin Poulain</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - AffectsNextSibling style relation marking is inefficient"
href="https://bugs.webkit.org/show_bug.cgi?id=156593">bug 156593</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #276426 Flags</td>
<td>review?
</td>
<td>review+
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - AffectsNextSibling style relation marking is inefficient"
href="https://bugs.webkit.org/show_bug.cgi?id=156593#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - AffectsNextSibling style relation marking is inefficient"
href="https://bugs.webkit.org/show_bug.cgi?id=156593">bug 156593</a>
from <span class="vcard"><a class="email" href="mailto:benjamin@webkit.org" title="Benjamin Poulain <benjamin@webkit.org>"> <span class="fn">Benjamin Poulain</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=276426&action=diff" name="attach_276426" title="patch">attachment 276426</a> <a href="attachment.cgi?id=276426&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=276426&action=review">https://bugs.webkit.org/attachment.cgi?id=276426&action=review</a>
<span class="quote">> Source/WebCore/css/SelectorChecker.cpp:94
> + if (last.type == Style::Relation::AffectsNextSibling && last.element->previousElementSibling() == &element) {</span >
It would likely be better to do:
element ->nextElementSibling() == last.element
to avoid touching the memory of last.element again.
<span class="quote">> Source/WebCore/cssjit/SelectorCompiler.cpp:2214
> + LocalRegister lastRelation(m_registerAllocator);
> + m_assembler.load32(sizeAddress, lastRelation);</span >
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.
<span class="quote">> Source/WebCore/cssjit/SelectorCompiler.cpp:2237
> + m_assembler.load32(valueAddress, value);
> + m_assembler.add32(Assembler::TrustedImm32(1), value);
> + m_assembler.store32(value, valueAddress);</span >
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.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>