On Mon, Aug 5, 2013 at 6:44 PM, Mike Stump <[email protected]> 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
