Re: [patch] Fix PR 22493

[Diego Novillo <dnovillo at redhat dot com>]

On Sun, Jul 24, 2005 at 03:16:14AM -0400, James A. Morrison wrote:

> 	* tree-vrp.c (extract_range_from_unary_expr): Deal with -fwrapv and
> 	VR_ANTI_RANGEs properly for NEGATE_EXPRs and ABS_EXPRs.
> 
Thanks.  It needs a few changes before it can go in:

> @@ -1353,27 +1356,64 @@ extract_range_from_unary_expr (value_ran
>    if (code == NEGATE_EXPR
>        && !TYPE_UNSIGNED (TREE_TYPE (expr)))
>      {
> -      /* Negating an anti-range doesn't really do anything to it.  The
> -	 new range will also not take on the same range of values
> -	 excluded by the original anti-range.  */
> +      /* Negating an anti-range doesn't really do anything to it if the max
> +	 value is equal to the min value.  Then the new range will also not
> +	 take on the same range of values excluded by the original
> +	 anti-range.  */
>        if (vr0.type == VR_ANTI_RANGE)
>  	{
> -	  copy_value_range (vr, &vr0);
> +	  tree neg_min = fold_unary_to_constant (code, TREE_TYPE (expr),
> +						 vr0.min);
> +	  if (operand_equal_p (vr0.max, neg_min, 0))
> +	    copy_value_range (vr, &vr0);
> +	  else
> +	    set_value_range_to_varying (vr);
>  	  return;
>  	}
>
I think both the original code and this fix get it wrong.
Thinking better about anti-ranges, I don't think there's anything
special about them wrt NEGATE_EXPR:

		a_4 		-> ~[1, 9]
		a_5 = -a_4	-> ~[-9, -1]

		a_4		-> ~[2, 2]
		a_5 = -a_4	-> ~[-2, -2]

This is exactly what we do for regular ranges.  Just flip the
range around and apply NEGATE_EXPR to each end.  I don't know
what I was thinking when I decided to keep the original
anti-range, but it couldn't be right.

>  
> -      /* NEGATE_EXPR flips the range around.  */
> -      min = (vr0.max == TYPE_MAX_VALUE (TREE_TYPE (expr)))
> -	    ? TYPE_MIN_VALUE (TREE_TYPE (expr))
> -	    : fold_unary_to_constant (code, TREE_TYPE (expr), vr0.max);
> +      if (flag_wrapv && vr0.min == TYPE_MIN_VALUE (TREE_TYPE (expr)))
> +	{
> +	  tree one = build_int_cst (TREE_TYPE (expr), 1);
>
Document the handling of -fwrapv here.

>    else if (code == ABS_EXPR
> -           && !TYPE_UNSIGNED (TREE_TYPE (expr)))
> +           && !(TYPE_UNSIGNED (TREE_TYPE (expr))))
>
Not needed.

>      {
> +      int comp;
> +      /* Deal with VR_ANTI_RANGEs in the same way we do for NEGATE_EXPRs.  */
> +      if (vr_type == VR_ANTI_RANGE)
> +	{
> +	  tree neg_min = fold_unary_to_constant (NEGATE_EXPR, TREE_TYPE (expr),
> +						 vr0.min);
> +	  if (operand_equal_p (vr0.max, neg_min, 0))
> +	    copy_value_range (vr, &vr0);
> +	  else
> +	    set_value_range_to_varying (vr);
> +	  return;
> +	}
> +
Hmm, no.  Just ask if range_includes_zero_p (&vr0).  If so, set VR
to VARYING.  We know that ABS_EXPR <~[-N1, -N2]> will be ~[N2, N1].
But if the range includes 0, say ~[-3, 1], we would end with ~[1, 3].
Which would be wrong for input 2.

If the range does not include zero, handle it as a regular range.

> @@ -1382,12 +1422,22 @@ extract_range_from_unary_expr (value_ran
>  
>        max = fold_unary_to_constant (code, TREE_TYPE (expr), vr0.max);
>  
> -      /* If the range was reversed, swap MIN and MAX.  */
> -      if (compare_values (min, max) == 1)
> +      comp = compare_values (min, max);
> +      if (tree_int_cst_sgn (vr0.min) == tree_int_cst_sgn (vr0.max))
>
Why do you need this?