On Mon, Aug 5, 2013 at 6:44 PM, Mike Stump <mikest...@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

Reply via email to