Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

On 09/20/16 16:20, Kyrill Tkachov wrote:
>
> On 20/09/16 15:11, Bernd Edlinger wrote:
>> On 09/20/16 13:29, Kyrill Tkachov wrote:
>>> arm bootstrap is now failing:
>>> $SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
>>> boolean context [-Werror=int-in-bool-context]
>>>       : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>>                              ~~~~~~~~~~~~~^~~~~~~~~~
>>> $SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
>>> 'TARGET_ARM_FP'
>>>      if (TARGET_ARM_FP)
>>>
>>>
>>> The full definition of TARGET_ARM_FP is:
>>> #define TARGET_ARM_FP            \
>>>     (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4        \
>>>               : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>>                 : 0)
>>>
>>> We want it set to 0 when there's no FP but when FP is available we set
>>> it to a bitmask
>>> to suggest the level that is available. That seems like a legitimate use
>>> to me.
>>>
>> Ok, I see, sorry for that.
>>
>> I think I will have to suppress the warning if the conditional is in
>> a macro somehow.
>>
>> Can you work around that for a few days?
>
> I changed to:
>   if (TARGET_ARM_FP != 0)
> in my tree to allow the build to proceed but encountered another
> bootstrap failure:
> In file included from ./tm.h:38:0,
>                   from $SRC/gcc/backend.h:28,
>                   from $SRC/gcc/regrename.c:23:
> $SRC/gcc/regrename.c: In function 'void rename_chains()':
> $SRC/gcc/config/arm/arm.h:915:4: error: ?: using integer constants in
> boolean context [-Werror=int-in-bool-context]
>     (TARGET_ARM     \
>     ~~~~~~~~~~~~~~~~~
>      ? ARM_HARD_FRAME_POINTER_REGNUM  \
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      : THUMB_HARD_FRAME_POINTER_REGNUM)
>      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> $SRC/gcc/regrename.c:484:8: note: in expansion of macro
> 'HARD_FRAME_POINTER_REGNUM'
>      || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
>          ^~~~~~~~~~~~~~~~~~~~~~~~~
> That looks like a legitimate bug in regrename.c ?
> The full condition is:
>         if (fixed_regs[reg] || global_regs[reg]
>             || (!HARD_FRAME_POINTER_IS_FRAME_POINTER &&
> frame_pointer_needed
>                 && reg == HARD_FRAME_POINTER_REGNUM)
>             || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
>                 && reg == FRAME_POINTER_REGNUM))
>
> The condition in the second || looks bogus (what use testing if a
> register is 'non-zero').
>
> So this looks like a useful catch by the warning, though I'm not sure at
> first glance how
> to fix it.
> Kyrill
>

I assume HARD_FRAME_POINTER_REGNUM is never zero.

That looks like it should be

  || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)


Bernd.