[Mesa-dev] [PATCH excerpt] mesa: Wrap unlikely() around conditions leading to _mesa_error().

[Eric Anholt]

Brian Paul <brianp at vmware.com> writes:

> On 03/02/2015 04:03 PM, Eric Anholt wrote:
>> Kenneth Graunke <kenneth at whitecape.org> writes:
>>
>>> Generated by the following Coccinelle semantic patch:
>>>
>>> @@
>>> expression E;
>>> @@
>>> if (
>>> -E
>>> +unlikely(E)
>>>   )
>>> { ... _mesa_error(...) ... }
>>>
>>> Improves performance in OglBatch7 (a CPU-bound microbenchmark) on
>>> an i7 4750HQ (Haswell) by 1.08546% +/- 0.519734% (n=292).
>>>
>>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>>
>> Neat!
>>
>> I think you're supposed to be able to get the same result by marking
>> _mesa_error as attribute cold.  Then nobody can forget to mark their
>> conditions as unlikely in the future.
>
> I suspect unlikely() would be supported by more compilers than 'cold'.

gcc does it since 4.3, at least, and apparently clang does too.  It
means that we don't have to reply to everyone's patches in the future to
say "hey, you forget to put unlikely() around your conditions leading to
_mesa_error()".  And it automatically gets code-size optimization for
things like _mesa_ColorTableParameterfv(), and should get the goto to
fog.c's invalid_pname as well.

If some important compiler has __builtin_expect but not cold, it might
make sense to land this still, along with attribute cold.  But there's
just no chance we're going to remember to unlikely() every condition
leading to _mesa_error(), and this coccinnelle patch isn't enough to get
all the win we can.

(this is all assuming that attribute cold actually works as advertised)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150302/ba9540b4/attachment.sig>