Re: [PATCH v2] fpu/softfloat: Silent 'bitwise negation of a boolean expression' warning

[Eric Blake]

On 5/28/20 4:00 AM, Philippe Mathieu-Daudé wrote:
> On 5/28/20 10:57 AM, Thomas Huth wrote:
> > On 28/05/2020 10.48, Philippe Mathieu-Daudé wrote:
> > > When building with clang version 10.0.0-4ubuntu1, we get:

In the subject, I'd suggest s/Silent/Silence/

> > >      CC      lm32-softmmu/fpu/softfloat.o
> > >    fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; 
> > > did you mean logical negation? [-Werror,-Wbool-operation]
> > >        absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
> > >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > >    fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; 
> > > did you mean logical negation? [-Werror,-Wbool-operation]
> > >            absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
> > >                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Also, do you need to list all errors, or will just one or two 
representative errors be sufficient?

> > > Fix by rewriting the fishy bitwise AND of two bools as an int.
> > >
> > > Suggested-by: Eric Blake <eblake@redhat.com>
> > > Buglink: https://bugs.launchpad.net/bugs/1881004
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > ---

> > > +++ b/fpu/softfloat.c
> > > @@ -3362,7 +3362,9 @@ static int32_t roundAndPackInt32(bool zSign, uint64_t 
> > > absZ,
> > >       }
> > >       roundBits = absZ & 0x7F;
> > >       absZ = ( absZ + roundIncrement )>>7;
> > > -    absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
> > > +    if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
> > > +        absZ &= ~1;
> > > +    }
> >
> > You could get rid of some more parentheses now:
> >
> >     if ((roundBits ^ 0x40) == 0 && roundNearestEven)
> >
> > ... also in the other hunks.
>
> I first wrote
>
>      if (!(roundBits ^ 0x40) && roundNearestEven)
>
> But then thought this would diverge from Eric suggestion, so I kept what
> he wrote (which is a bit closer to the style of rest of this file).

I don't mind the patch as-is for minimizing churn and matching existing 
style, but I also would not be opposed if you wanted to elide 
unnecessary ().

> > Anyway:
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> Thanks!

Reviewed-by: Eric Blake <eblake@redhat.com>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org