Re: all_ones_mask_p clarification

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Aug 5, 2013 at 6:44 PM, Mike Stump <mikestump@comcast.net> wrote:
> It is the intent for all_ones_mask_p to return true when 64 bits of ones in an unsigned type of width 64 when size is 64, right?  Currently the code uses a signed type for tmask, which sets the upper bits to 1, when the value includes the sign bit set and the equality code does check all 128 bits of the the value for equality.  This results in the current code returning false in this case.  The below change is the behavior change I'm talking about.
>
> We're fixing this in the wide-int branch, and just wanted to see if someone wanted to argue this isn't a bug.
>
> If you want to see a small test case:
>
> typedef enum
> {
>   DK_ERROR,
>   DK_SORRY,
>   DK_LAST_DIAGNOSTIC_KIND
> } diagnostic_t;
>
> struct diagnostic_context
> {
>   int diagnostic_count[DK_LAST_DIAGNOSTIC_KIND];
>   diagnostic_t *classify_diagnostic;
> };
>
> extern diagnostic_context *global_dc;
>
> bool
> seen_error (void)
> {
>   return (global_dc)->diagnostic_count[(int) (DK_ERROR)] || (global_dc)->diagnostic_count[(int) (DK_SORRY)];
> }
>
>
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 6506ae7..9b17d1d 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -3702,12 +3702,23 @@ all_ones_mask_p (const_tree mask, int size)
>
>    tmask = build_int_cst_type (signed_type_for (type), -1);
>
> -  return
> -    tree_int_cst_equal (mask,
> -                       const_binop (RSHIFT_EXPR,
> -                                    const_binop (LSHIFT_EXPR, tmask,
> -                                                 size_int (precision - size)),
> -                                    size_int (precision - size)));
> +  if (tree_int_cst_equal (mask,
> +                         const_binop (RSHIFT_EXPR,
> +                                      const_binop (LSHIFT_EXPR, tmask,
> +                                                   size_int (precision - size)),
> +                                      size_int (precision - size))))
> +    return true;
> +
> +  tmask = build_int_cst_type (unsigned_type_for (type), -1);
> +
> +  if (tree_int_cst_equal (mask,
> +                         const_binop (RSHIFT_EXPR,
> +                                      const_binop (LSHIFT_EXPR, tmask,
> +                                                   size_int (precision - size)),
> +                                      size_int (precision - size))))
> +    return true;
> +
> +  return false;

This should instead use

   return tree_to_double_int (mask) == double_int::mask (size)
     || (TYPE_PRECISION (mask) == size && tree_to_double_int
(mask).all_onesp ())

and avoid all the tree building.

Richard.

>  }
>
>  /* Subroutine for fold: determine if VAL is the INTEGER_CONST that