On Fri, May 17, 2013 at 5:51 PM, Jeff Law <l...@redhat.com> wrote:
> As I believe I pointed out in a follow-up message, 254.gap is depending on
> signed overflow semantics.
>
> This patch avoids eliminating a cast feeding a conditional when the
> SSA_NAME's range has overflowed unless -fstrict-overflow is in effect. Thus
> 254.gap should be building with -fno-strict-overflow.
>
> This patch also introduces a warning when the optimization is applied and
> the ranges have overflowed.
>
> Bootstrapped and regression tested on x86-unknown-linux-gnu.
>
> OK for the trunk?
>
>
>
>
> commit 62bbaa8de0e8d929eb3c63331b47950e9b09d801
> Author: Jeff Law <l...@redhat.com>
> Date: Wed May 1 12:33:20 2013 -0600
>
> PR tree-optimization/57124
> * tree-vrp.c (simplify_cond_using_ranges): Only simplify a
> conversion feeding a condition if the range has an overflow
> if -fstrict-overflow. Add warnings for when we do make the
> transformation.
>
> PR tree-optimization/57124
> * gcc.c-torture/execute/pr57124.c: New test.
> * gcc.c-torture/execute/pr57124.x: Set -fno-strict-overflow.
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 8e92c44..9320f21 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,11 @@
> +2013-05-17 Jeff Law <l...@redhat.com>
> +
> + PR tree-optimization/57124
> + * tree-vrp.c (simplify_cond_using_ranges): Only simplify a
> + conversion feeding a condition if the range has an overflow
> + if -fstrict-overflow. Add warnings for when we do make the
> + transformation.
> +
> 2013-05-16 Rainer Orth <r...@cebitec.uni-bielefeld.de>
>
> * reorg.c (link_cc0_insns): Wrap in #ifdef HAVE_cc0.
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 879b9bc..482151c 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2013-05-17 Jeff Law <l...@redhat.com>
> +
> + PR tree-optimization/57124
> + * gcc.c-torture/execute/pr57124.c: New test.
> + * gcc.c-torture/execute/pr57124.x: Set -fno-strict-overflow.
> +
> 2013-05-16 Greta Yorsh <greta.yo...@arm.com>
>
> * gcc.target/arm/unaligned-memcpy-2.c: Adjust expected output.
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr57124.c
> b/gcc/testsuite/gcc.c-torture/execute/pr57124.c
> new file mode 100644
> index 0000000..835d249
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr57124.c
> @@ -0,0 +1,27 @@
> +__attribute__ ((noinline))
> +foo(short unsigned int *p1, short unsigned int *p2)
> +{
> + short unsigned int x1, x4;
> + int x2, x3, x5, x6;
> + unsigned int x7;
> +
> + x1 = *p1;
> + x2 = (int) x1;
> + x3 = x2 * 65536;
> + x4 = *p2;
> + x5 = (int) x4;
> + x6 = x3 + x4;
> + x7 = (unsigned int) x6;
> + if (x7 <= 268435455U)
> + abort ();
> + exit (0);
> +}
> +
> +main()
> +{
> + short unsigned int x, y;
> + x = -5;
> + y = -10;
> + foo (&x, &y);
> +}
> +
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr57124.x
> b/gcc/testsuite/gcc.c-torture/execute/pr57124.x
> new file mode 100644
> index 0000000..d8cacbe
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr57124.x
> @@ -0,0 +1,2 @@
> +set additional_flags "-fno-strict-overflow"
> +return 0
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index b5de683..b3eccf0 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -8669,8 +8669,32 @@ simplify_cond_using_ranges (gimple stmt)
> && range_fits_type_p (vr,
> TYPE_PRECISION (TREE_TYPE (op0)),
> TYPE_UNSIGNED (TREE_TYPE (op0)))
> - && int_fits_type_p (op1, TREE_TYPE (innerop)))
> + && int_fits_type_p (op1, TREE_TYPE (innerop))
> + /* The range must not have overflowed, or if it did overflow
> + we must not be wrapping/trapping overflow and optimizing
> + with strict overflow semantics. */
> + && ((!is_negative_overflow_infinity (vr->min)
> + && !is_positive_overflow_infinity (vr->max))
> + || (!flag_wrapv && !flag_trapv && flag_strict_overflow)))
Ok with changing this to
|| TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (innerop)))
> {
> + /* If the range overflowed and the user has asked for warnings
> + when strict overflow semantics were used to optimize code,
> + issue an appropriate warning. */
> + if ((is_negative_overflow_infinity (vr->min)
> + || is_positive_overflow_infinity (vr->max))
> + && issue_strict_overflow_warning
> (WARN_STRICT_OVERFLOW_CONDITIONAL))
> + {
> + location_t location;
> +
> + if (!gimple_has_location (stmt))
> + location = input_location;
> + else
> + location = gimple_location (stmt);
> + warning_at (location, OPT_Wstrict_overflow,
> + "assuming signed overflow does not occur when "
> + "simplifying conditional.");
and the trailing '.' removed
Thanks,
Richard.
> + }
> +
> tree newconst = fold_convert (TREE_TYPE (innerop), op1);
> gimple_cond_set_lhs (stmt, innerop);
> gimple_cond_set_rhs (stmt, newconst);
>
