Re: [PATCH] fold-const: Fix up make_range_step [PR105189]

[Richard Biener via Gcc-patches]

> Am 07.04.2022 um 17:47 schrieb Jakub Jelinek via Gcc-patches 
> <gcc-patches@gcc.gnu.org>:
> 
> Hi!
> 
> The following testcase is miscompiled, because fold_truth_andor
> incorrectly folds
> (unsigned) foo () >= 0U && 1
> into
> foo () >= 0
> For the unsigned comparison (which is useless in this case,
> as >= 0U is always true, but hasn't been folded yet), previous
> make_range_step derives exp (unsigned) foo () and
> +[0U, -]
> range for it.  Next we process the NOP_EXPR.  We have special code
> for unsigned to signed casts, already earlier punt if low or high
> aren't representable in arg0_type or if it is a narrowing conversion.
> For the signed to unsigned casts, I think if high is specified we
> are still fine, as we punt for non-representable values in arg0_type,
> n_high is then still representable and so was smaller or equal to
> signed maximum and either low is not present (equivalent to 0U), or
> low must be smaller or equal to high and so for unsigned exp
> +[low, high] the signed exp +[n_low, n_high] will be correct.
> Similarly, if both low and high aren't specified (always true or
> always false), it is ok too.
> But if we have for unsigned exp +[low, -] or -[low, -], using
> +[n_low, -] or -[n_high, -] is incorrect.  Because low is smaller
> or equal to signed maximum and high is unspecified (i.e. unsigned
> maximum), when signed that range is a union of +[n_low, -] and
> +[-, -1] which is equivalent to -[0, n_low-1], unless low
> is 0, in that case we can treat it as [-, -].
> 
> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

Ok.

Thanks,
Richard 

> 2022-04-07  Jakub Jelinek  <ja...@redhat.com>
> 
>    PR tree-optimization/105189
>    * fold-const.cc (make_range_step): Fix up handling of
>    (unsigned) x +[low, -] ranges for signed x if low fits into
>    typeof (x).
> 
>    * g++.dg/torture/pr105189.C: New test.
> 
> --- gcc/fold-const.cc.jj    2022-04-04 12:09:00.317116906 +0200
> +++ gcc/fold-const.cc    2022-04-07 11:32:41.185313269 +0200
> @@ -5212,7 +5212,7 @@ make_range_step (location_t loc, enum tr
>    n_high = fold_convert_loc (loc, arg0_type, n_high);
> 
>       /* If we're converting arg0 from an unsigned type, to exp,
> -     a signed type,  we will be doing the comparison as unsigned.
> +     a signed type, we will be doing the comparison as unsigned.
>     The tests above have already verified that LOW and HIGH
>     are both positive.
> 
> @@ -5274,6 +5274,32 @@ make_range_step (location_t loc, enum tr
>        }
>    }
> 
> +      /* Otherwise, if we are converting arg0 from signed type, to exp,
> +     an unsigned type, we will do the comparison as signed.  If
> +     high is non-NULL, we punt above if it doesn't fit in the signed
> +     type, so if we get through here, +[-, high] or +[low, high] are
> +     equivalent to +[-, n_high] or +[n_low, n_high].  Similarly,
> +     +[-, -] or -[-, -] are equivalent too.  But if low is specified and
> +     high is not, the +[low, -] range is equivalent to union of
> +     +[n_low, -] and +[-, -1] ranges, so +[low, -] is equivalent to
> +     -[0, n_low-1] and similarly -[low, -] to +[0, n_low-1], except for
> +     low being 0, which should be treated as [-, -].  */
> +      else if (TYPE_UNSIGNED (exp_type)
> +           && !TYPE_UNSIGNED (arg0_type)
> +           && low
> +           && !high)
> +    {
> +      if (integer_zerop (low))
> +        n_low = NULL_TREE;
> +      else
> +        {
> +          n_high = fold_build2_loc (loc, PLUS_EXPR, arg0_type,
> +                    n_low, build_int_cst (arg0_type, -1));
> +          n_low = build_zero_cst (arg0_type);
> +          in_p = !in_p;
> +        }
> +    }
> +
>       *p_low = n_low;
>       *p_high = n_high;
>       *p_in_p = in_p;
> --- gcc/testsuite/g++.dg/torture/pr105189.C.jj    2022-04-07 
> 11:40:06.195098778 +0200
> +++ gcc/testsuite/g++.dg/torture/pr105189.C    2022-04-07 11:39:50.177322461 
> +0200
> @@ -0,0 +1,19 @@
> +// PR tree-optimization/105189
> +// { dg-do run }
> +
> +int
> +foo ()
> +{
> +  return -1;
> +}
> +
> +int
> +main ()
> +{
> +  int c = foo () >= 0U && 1;
> +  if (c != 1)
> +    __builtin_abort ();
> +  int d = foo () >= 3U && 1;
> +  if (d != 1)
> +    __builtin_abort ();
> +}
> 
>    Jakub
>