On Mon, May 19, 2014 at 12:19 PM, Richard Biener <rguent...@suse.de> wrote:
> On Fri, 16 May 2014, Jeff Law wrote:
>
>> On 05/14/14 03:06, Richard Biener wrote:
>> >
>> > The following fixes pre/post-inc/dec gimplification of promoted
>> > integer types. There is the issue with the way TYPE_OVERFLOW_UNDEFINED
>> > is related to TYPE_OVERFLOW_WRAPS and the (non-)semantics of
>> > -fno-strict-overflow.
>> >
>> > In this case, with -On -fno-strict-overflow for a variable of
>> > type short we have !TYPE_OVERFLOW_WRAPS _and_ !TYPE_OVERFLOW_UNDEFINED
>> > (so we're in an "undefined" area). Which means that
>> > !TYPE_OVERFLOW_UNDEFINED doesn't imply that overflow wraps.
>> >
>> > Thus the gimplification has to play on the safe side and
>> > always use an unsigned type unless the user specifies -fwrapv
>> > (the flag with a proper semantic meaning).
>> >
>> > That is, it seems to be the case that what predicate to use
>> > (TYPE_OVERFLOW_WRAPS or TYPE_OVERFLOW_UNDEFINED, independent
>> > on whether you invert it), depends on the use-case in a very
>> > awkward (and error-prone) way.
>> >
>> > Bootstrap and regtest pending on x86_64-unknown-linux-gnu, ok
>> > if that succeeds (I expect to have to adjust some testcases)?
>> >
>> > Thanks,
>> > Richard.
>> >
>> > 2014-05-14 Richard Biener <rguent...@suse.de>
>> >
>> > c-family/
>> > * c-gimplify.c (c_gimplify_expr): Gimplify self-modify expressions
>> > using unsigned arithmetic if overflow does not wrap instead of
>> > if overflow is undefined.
>> >
>> > * c-c++-common/torture/pr61184.c: New testcase.
>> Seems reasonable to me.
>
> If it is then I'd strongly suggest to make -fno-strict-overflow
> imply -fwrapv. Otherwise for -fno-strict-overflow we can
> neither rely on signed arithmetic wrapping nor rely on it
> invoking undefined behavior - instead it's in lala-land as far
> as the middle-end is concerned (and get's us the worst of
> both -fwrapv and -fno-wrapv).
>
> Well, it turns out after re-visiting the bug, that the issue
> lies in VRP instead as it doesn't detect [0, +INF] -> [0, +INF(OVF)]
> as a lattice change and thus it misses visiting dependent
> stmts again.
But it turns out the original patch is still necessary to fix PR61741,
thus re-bootstrapped and tested and installed.
Richard.
> Bootstrap and regtest in progress on x86_64-unknown-linux-gnu.
>
> Thanks,
> Richard.
>
> 2014-05-19 Richard Biener <rguent...@suse.de>
>
> PR tree-optimization/61184
> * tree-vrp.c (is_negative_overflow_infinity): Use
> TREE_OVERFLOW_P and do that check first.
> (is_positive_overflow_infinity): Likewise.
> (is_overflow_infinity): Likewise.
> (vrp_operand_equal_p): Properly treat operands with
> differing overflow as not equal.
>
> * c-c++-common/torture/pr61184.c: New testcase.
>
> Index: gcc/tree-vrp.c
> ===================================================================
> *** gcc/tree-vrp.c (revision 210607)
> --- gcc/tree-vrp.c (working copy)
> *************** positive_overflow_infinity (tree type)
> *** 293,301 ****
> static inline bool
> is_negative_overflow_infinity (const_tree val)
> {
> ! return (needs_overflow_infinity (TREE_TYPE (val))
> ! && CONSTANT_CLASS_P (val)
> ! && TREE_OVERFLOW (val)
> && vrp_val_is_min (val));
> }
>
> --- 293,300 ----
> static inline bool
> is_negative_overflow_infinity (const_tree val)
> {
> ! return (TREE_OVERFLOW_P (val)
> ! && needs_overflow_infinity (TREE_TYPE (val))
> && vrp_val_is_min (val));
> }
>
> *************** is_negative_overflow_infinity (const_tre
> *** 304,312 ****
> static inline bool
> is_positive_overflow_infinity (const_tree val)
> {
> ! return (needs_overflow_infinity (TREE_TYPE (val))
> ! && CONSTANT_CLASS_P (val)
> ! && TREE_OVERFLOW (val)
> && vrp_val_is_max (val));
> }
>
> --- 303,310 ----
> static inline bool
> is_positive_overflow_infinity (const_tree val)
> {
> ! return (TREE_OVERFLOW_P (val)
> ! && needs_overflow_infinity (TREE_TYPE (val))
> && vrp_val_is_max (val));
> }
>
> *************** is_positive_overflow_infinity (const_tre
> *** 315,323 ****
> static inline bool
> is_overflow_infinity (const_tree val)
> {
> ! return (needs_overflow_infinity (TREE_TYPE (val))
> ! && CONSTANT_CLASS_P (val)
> ! && TREE_OVERFLOW (val)
> && (vrp_val_is_min (val) || vrp_val_is_max (val)));
> }
>
> --- 313,320 ----
> static inline bool
> is_overflow_infinity (const_tree val)
> {
> ! return (TREE_OVERFLOW_P (val)
> ! && needs_overflow_infinity (TREE_TYPE (val))
> && (vrp_val_is_min (val) || vrp_val_is_max (val)));
> }
>
> *************** vrp_operand_equal_p (const_tree val1, co
> *** 791,799 ****
> return true;
> if (!val1 || !val2 || !operand_equal_p (val1, val2, 0))
> return false;
> ! if (is_overflow_infinity (val1))
> ! return is_overflow_infinity (val2);
> ! return true;
> }
>
> /* Return true, if the bitmaps B1 and B2 are equal. */
> --- 788,794 ----
> return true;
> if (!val1 || !val2 || !operand_equal_p (val1, val2, 0))
> return false;
> ! return is_overflow_infinity (val1) == is_overflow_infinity (val2);
> }
>
> /* Return true, if the bitmaps B1 and B2 are equal. */
> Index: gcc/testsuite/c-c++-common/torture/pr61184.c
> ===================================================================
> *** gcc/testsuite/c-c++-common/torture/pr61184.c (revision 0)
> --- gcc/testsuite/c-c++-common/torture/pr61184.c (revision 0)
> ***************
> *** 0 ****
> --- 1,18 ----
> + /* { dg-do run } */
> + /* { dg-additional-options "-fno-strict-overflow" } */
> +
> + short a;
> +
> + void
> + foo (void)
> + {
> + for (a = 0; a >= 0; a++)
> + ;
> + }
> +
> + int
> + main ()
> + {
> + foo ();
> + return 0;
> + }
