From: Jani Nikula <jani.nikula@linux.intel.com>
To: John Harrison <john.c.harrison@intel.com>,
Intel-GFX@Lists.FreeDesktop.Org
Cc: DRI-Devel@Lists.FreeDesktop.Org
Subject: Re: [Intel-gfx] [PATCH v2 1/8] drm/i915/guc: Do not conflate lrc_desc with GuC id for registration
Date: Tue, 08 Mar 2022 12:48:46 +0200 [thread overview]
Message-ID: <87o82g1zzl.fsf@intel.com> (raw)
In-Reply-To: <c6aeece0-9b12-e135-c991-25793f7640bb@intel.com>
On Fri, 04 Mar 2022, John Harrison <john.c.harrison@intel.com> wrote:
> On 3/4/2022 03:59, Jani Nikula wrote:
>> On Thu, 24 Feb 2022, John.C.Harrison@Intel.com 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
next prev parent reply other threads:[~2022-03-08 10:49 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-25 0:06 [Intel-gfx] [PATCH v2 0/8] Prep work for next GuC release John.C.Harrison
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 1/8] drm/i915/guc: Do not conflate lrc_desc with GuC id for registration John.C.Harrison
2022-03-04 11:59 ` Jani Nikula
2022-03-04 20:17 ` John Harrison
2022-03-08 10:48 ` Jani Nikula [this message]
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 2/8] drm/i915/guc: Add an explicit 'submission_initialized' flag John.C.Harrison
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 3/8] drm/i915/guc: Better name for context id limit John.C.Harrison
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 4/8] drm/i915/guc: Split guc_lrc_desc_pin apart John.C.Harrison
2022-02-26 0:10 ` Ceraolo Spurio, Daniele
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 5/8] drm/i915/guc: Move lrc desc setup to where it is needed John.C.Harrison
2022-02-26 0:12 ` Ceraolo Spurio, Daniele
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 6/8] drm/i915/guc: Rename desc_idx to ctx_id John.C.Harrison
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 7/8] drm/i915/guc: Drop obsolete H2G definitions John.C.Harrison
2022-02-25 0:06 ` [Intel-gfx] [PATCH v2 8/8] drm/i915/guc: Fix potential invalid pointer dereferences when decoding G2Hs John.C.Harrison
2022-02-25 4:13 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Prep work for next GuC release (rev3) Patchwork
2022-02-25 4:43 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-02-28 19:30 ` John Harrison
2022-02-25 21:03 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87o82g1zzl.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=DRI-Devel@Lists.FreeDesktop.Org \
--cc=Intel-GFX@Lists.FreeDesktop.Org \
--cc=john.c.harrison@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox