Re: [PATCH] xen: Rework WARN_ON() to return whether a warning was triggered

[Julien Grall <julien@xxxxxxx>]

Hi,

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(...) )

Cheers,

> Jan
--
Julien Grall