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 <ebl...@redhat.com> >>>> Buglink: https://bugs.launchpad.net/bugs/1881004 >>>> Signed-off-by: Philippe Mathieu-Daudé <phi...@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 <th...@redhat.com> >>> >> >> Thanks! >> > > Reviewed-by: Eric Blake <ebl...@redhat.com> >
