Re: [PATCH v2 1/8] drm/i915/guc: Do not conflate lrc_desc with GuC id for registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 04 Mar 2022, John Harrison <john.c.harrison@xxxxxxxxx> wrote:
> On 3/4/2022 03:59, Jani Nikula wrote:
>> On Thu, 24 Feb 2022, John.C.Harrison@xxxxxxxxx wrote:
>> There are a plethora of static inlines in the guc .c files, and this
>> adds more. How about just letting the compiler decide what's the best
>> course of action, inline or not? I think hand rolling the inline is a
>> micro optimization that you'd need to justify i.e. show that you're
>> doing a better job than the compiler.
>>
>> The main downsides to using inlines are that you'll miss compiler
>> warnings for unused functions, and it sets an example for people to
>> start using inline more, while they should be an exception.
>>
>> BR,
>> Jani.
>>
>>
>> PS. I also don't much like the likely/unlikely annotations, but that's
>> another can of worms.
> Technically, this patch isn't adding any new ones. It is just reworking 
> existing functions in their existing style. So it basically comes under 
> your last point of people just following the prevailing style because 
> it's already there.
>
> I can add a task to the clean-up backlog to remove all mention of 
> inline. Not sure why you think the (un)likely tags are bad? Again, I 
> have no particular view either way.

The (un)likely annotations are similar to static inlines in that they're
often unjustified micro optimizations. Having plenty of them gives
people the false impression using them should be the rule rather than
the exception. And getting them wrong could have a high performance
penalty. They're certainly not meant for regular error handling.

Similar to static inlines, (un)likely have their uses, but they need to
be used sparingly and the use needs to be justified. For static inlines,
especially within .c files, just let the compiler do its job. For
(un)likely, just let the CPU branch predictor do its job.

The link's pretty old, but see also: https://lwn.net/Articles/420019/


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux