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

[Eric Blake <eblake@redhat.com>]

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