On Sat, Nov 21, 2015 at 7:57 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
> On Sat, 21 Nov 2015, Richard Biener wrote:
>
>> On November 20, 2015 8:58:15 PM GMT+01:00, Jason Merrill
>> <ja...@redhat.com> wrote:
>>>
>>> In this bug, we hit the (A & sign-bit) != 0 -> A < 0 transformation.
>>> Because of delayed folding, the operands aren't fully folded yet, so we
>>>
>>> have NOP_EXPRs around INTEGER_CSTs, and so calling wi::only_sign_bit_p
>>> ICEs. We've been seeing several similar bugs, where code calls
>>> integer_zerop and therefore assumes that they have an INTEGER_CST, but
>>> in fact integer_zerop does STRIP_NOPS.
>>>
>>> This patch changes the pattern to only match if the operand is actually
>>>
>>> an INTEGER_CST. Alternatively we could call tree_strip_nop_conversions
>>>
>>> on the operand, but I would expect that to have issues when the
>>> conversion changes the signedness of the type.
>>>
>>> OK if testing passes?
>>
>>
>> What happens if we remove the nops stripping from integer_zerop?
>
>
> I had the same reaction.
>
>> Do other integer predicates strip nops?
>
>
> Yes, they do.
>
> I believe I added one or two of those, and the reason I added STRIP_NOPS is
> because they started as a copy-paste of integer_zerop...
Ok...
Jason, from looking at the PRs backtrace I see the C++ FE does things like
if (complain & tf_warning)
warn_logical_operator (loc, code, boolean_type_node,
code_orig_arg1, fold (arg1),
code_orig_arg2, fold (arg2));
but that's in principle a no-no, if arg1s operands are not folded.
Delayed folding needs
to happen recursively, bottom-up. Folders generally do not expect
unfolded operands
like (int) 1.
There is c-common.c:c_fully_fold () which does this properly but with
/* This function is not relevant to C++ because C++ folds while
parsing, and may need changes to be correct for C++ when C++
stops folding while parsing. */
if (c_dialect_cxx ())
gcc_unreachable ();
not sure if the C++ FE can re-use this for the diagnostic cases.
Richard.
> --
> Marc Glisse
