On Tue, 15 Dec 2020, Jan Beulich wrote:
> On 15.12.2020 14:19, Julien Grall wrote:
> > On 15/12/2020 11:46, Jan Beulich wrote:
> >> On 15.12.2020 12:26, Julien Grall wrote:
> >>> --- a/xen/include/xen/lib.h
> >>> +++ b/xen/include/xen/lib.h
> >>> @@ -23,7 +23,13 @@
> >>> #include <asm/bug.h>
> >>>
> >>> #define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
> >>> -#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
> >>> +#define WARN_ON(p) ({ \
> >>> + bool __ret_warn_on = (p); \
> >>
> >> Please can you avoid leading underscores here?
> >
> > I can.
> >
> >>
> >>> + \
> >>> + if ( unlikely(__ret_warn_on) ) \
> >>> + WARN(); \
> >>> + unlikely(__ret_warn_on); \
> >>> +})
> >>
> >> Is this latter unlikely() having any effect? So far I thought it
> >> would need to be immediately inside a control construct or be an
> >> operand to && or ||.
> >
> > The unlikely() is directly taken from the Linux implementation.
> >
> > My guess is the compiler is still able to use the information for the
> > branch prediction in the case of:
> >
> > if ( WARN_ON(...) )
>
> Maybe. Or maybe not. I don't suppose the Linux commit introducing
> it clarifies this?
I did a bit of digging but it looks like the unlikely has been there
forever. I'd just keep it as is.
