Date: Tue, 2 Jul 2024 12:05:52 -0600 From: Warner Losh <imp@bsdimp.com> To: John Baldwin <jhb@freebsd.org> Cc: Warner Losh <imp@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 2508372b7b46 - main - cdefs.h: Assume the compiler supports at least GNU C 3.0 extensions Message-ID: <CANCZdfo1W2yFSc2kCtZZ%2BN7f970upyRmrA3VivY9ctd5ax91vQ@mail.gmail.com> In-Reply-To: <7ca0ab94-4d5e-405e-b178-84ef3d8ebc8d@FreeBSD.org> References: <202406210241.45L2fNU2056962@gitrepo.freebsd.org> <7ca0ab94-4d5e-405e-b178-84ef3d8ebc8d@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] Hey John, On Mon, Jul 1, 2024 at 3:49 PM John Baldwin <jhb@freebsd.org> wrote: > On 6/20/24 7:41 PM, Warner Losh wrote: > > The branch main has been updated by imp: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=2508372b7b46117a9fb801b50624265d30888442 > > > > commit 2508372b7b46117a9fb801b50624265d30888442 > ... > > -/* > > - * GNU C version 2.96 adds explicit branch prediction so that > > - * the CPU back-end can hint the processor and also so that > > - * code blocks can be reordered such that the predicted path > > - * sees a more linear flow, thus improving cache behavior, etc. > > - * > > - * The following two macros provide us with a way to utilize this > > - * compiler feature. Use __predict_true() if you expect the expression > > - * to evaluate to true, and __predict_false() if you expect the > > - * expression to evaluate to false. > > - * > > - * A few notes about usage: > > - * > > - * * Generally, __predict_false() error condition checks (unless > > - * you have some _strong_ reason to do otherwise, in which case > > - * document it), and/or __predict_true() `no-error' condition > > - * checks, assuming you want to optimize for the no-error case. > > - * > > - * * Other than that, if you don't know the likelihood of a test > > - * succeeding from empirical or other `hard' evidence, don't > > - * make predictions. > > - * > > - * * These are meant to be used in places that are run `a lot'. > > - * It is wasteful to make predictions in code that is run > > - * seldomly (e.g. at subsystem initialization time) as the > > - * basic block reordering that this affects can often generate > > - * larger code. > > - */ > > -#if __GNUC_PREREQ__(2, 96) > > #define __predict_true(exp) __builtin_expect((exp), 1) > > #define __predict_false(exp) __builtin_expect((exp), 0) > > -#else > > -#define __predict_true(exp) (exp) > > -#define __predict_false(exp) (exp) > > -#endif > > I think the comment was worth keeping around. You just need to modify > the start of it to "Modern compilers include explicit branch prediction..." > In particular, I think our current practice is still to apply prediction > hints rather conservatively. > That's a fair point I hadn't considered. https://reviews.freebsd.org/D45837 adds it back with some tiny tweaks to the language, justification, etc. Warner [-- Attachment #2 --] <div dir="ltr"><div dir="ltr">Hey John,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jul 1, 2024 at 3:49 PM John Baldwin <<a href="mailto:jhb@freebsd.org">jhb@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 6/20/24 7:41 PM, Warner Losh wrote:<br> > The branch main has been updated by imp:<br> > <br> > URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=2508372b7b46117a9fb801b50624265d30888442" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=2508372b7b46117a9fb801b50624265d30888442</a><br>; > <br> > commit 2508372b7b46117a9fb801b50624265d30888442<br></blockquote><div>... </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> -/*<br> > - * GNU C version 2.96 adds explicit branch prediction so that<br> > - * the CPU back-end can hint the processor and also so that<br> > - * code blocks can be reordered such that the predicted path<br> > - * sees a more linear flow, thus improving cache behavior, etc.<br> > - *<br> > - * The following two macros provide us with a way to utilize this<br> > - * compiler feature. Use __predict_true() if you expect the expression<br> > - * to evaluate to true, and __predict_false() if you expect the<br> > - * expression to evaluate to false.<br> > - *<br> > - * A few notes about usage:<br> > - *<br> > - * * Generally, __predict_false() error condition checks (unless<br> > - * you have some _strong_ reason to do otherwise, in which case<br> > - * document it), and/or __predict_true() `no-error' condition<br> > - * checks, assuming you want to optimize for the no-error case.<br> > - *<br> > - * * Other than that, if you don't know the likelihood of a test<br> > - * succeeding from empirical or other `hard' evidence, don't<br> > - * make predictions.<br> > - *<br> > - * * These are meant to be used in places that are run `a lot'.<br> > - * It is wasteful to make predictions in code that is run<br> > - * seldomly (e.g. at subsystem initialization time) as the<br> > - * basic block reordering that this affects can often generate<br> > - * larger code.<br> > - */<br> > -#if __GNUC_PREREQ__(2, 96)<br> > #define __predict_true(exp) __builtin_expect((exp), 1)<br> > #define __predict_false(exp) __builtin_expect((exp), 0)<br> > -#else<br> > -#define __predict_true(exp) (exp)<br> > -#define __predict_false(exp) (exp)<br> > -#endif<br> <br> I think the comment was worth keeping around. You just need to modify<br> the start of it to "Modern compilers include explicit branch prediction..."<br> In particular, I think our current practice is still to apply prediction<br> hints rather conservatively.<br></blockquote><div><br></div><div>That's a fair point I hadn't considered. <a href="https://reviews.freebsd.org/D45837">https://reviews.freebsd.org/D45837</a></div><div>adds it back with some tiny tweaks to the language, justification, etc.</div><div><br></div><div>Warner </div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfo1W2yFSc2kCtZZ%2BN7f970upyRmrA3VivY9ctd5ax91vQ>