[PATCH] Mark highly predictable branches as likely/unlikely

[Michel Dänzer]

On Thu, 2010-04-08 at 09:43 +1000, Dave Airlie wrote: 
> On Wed, 2010-04-07 at 19:25 -0400, Matt Turner wrote:
> > On Wed, Apr 7, 2010 at 7:15 PM, Dave Airlie <airlied at redhat.com> wrote:
> > > On Wed, 2010-04-07 at 18:49 -0400, Matt Turner wrote:
> > >> Highly predicatble branches include
> > >>  - unlikely: error conditions, such as those leading to
> > >>       - RADEON_FALLBACK
> > >>       - goto fail
> > >>       - return FALSE (as an error
> > >>  - likely: if (info->useEXA)
> > >
> > > Can we limit these to fastpaths?
> > >
> > > i.e. no need in atombios or drmmode code or anywhere like that.
> > >
> > > I expect you just want it in the accel/exa/xv codepaths.
> > >
> > > Dave.
> > 
> > Sure. I can strip out the others and resend if you'd like, or you
> > could just commit the parts you think are appropriate. I don't imagine
> > there would be any ill effects from including the patch for files like
> >  src/atombios_output.c            |    2 +-
> >  src/drmmode_display.c            |    6 +-
> > though. This wasn't a blind find'n'replace, so the ones included
> > should be pretty good.
> 
> Its more for our sanity, and future maintainance cost. likely/unlikely
> isn't worth the effort, also these have to be maintained going forward,
> in case some path becomes more likely or unlikely.
> 
> The kernel has a real problem using these in the wrong places and I'd
> hate for it to spread.
> 
> like when the standard code was for kfree was
> 
> if (ptr)
> 	kfree(ptr)
> 
> and kfree contains an if (unlikely(!ptr)), then someone removed all the 
> if (ptr) and it suddenly became a lot more likely.

I share your concerns, and am not sure it's worth adding them even in
fastpaths given that the numbers seem to show barely any significant
improvement even now (how many gtkperf runs with how many iterations
were used to determine the numbers?).


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer