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

[Philippe Mathieu-Daudé]

On 5/28/20 3:37 PM, Eric Blake wrote:
> 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/

Don't fit... 73 chars :S I'll use 'Avoid' (or 'Remove'?) instead.

> 
>>>>
>>>>      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?

Fair :)

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

OK, thanks for the review!

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