On Thu, May 23, 2013 at 9:47 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
> Hello,
>
> this is a simple patch to reduce a bit the noise in PR57324 (undefined
> behavior flagged by clang). I only handled some of the most obvious ones.
> Passes bootstrap+testsuite on x86_64-linux-gnu.
Hm, so ISO C99 says in 6.5.7/4 that (E1 << E2) "If E1 has signed type
and nonnegative
value, and E1 * 2^E2 is representable in the result type, then that is the
resulting value; otherwise, the behavior is undefined."
While seriously underspecified for signed negative values (always undefined?!
or well-defined?!), I wonder why CLang requires
> - *hv &= ~((HOST_WIDE_INT) (-1) << (prec - count -
> HOST_BITS_PER_WIDE_INT));
> + *hv &= ~((unsigned HOST_WIDE_INT) (-1)
> + << (prec - count - HOST_BITS_PER_WIDE_INT));
here.
That is, isn't this a bug in CLang?
Richard.
>
> 2013-05-24 Marc Glisse <marc.gli...@inria.fr>
>
> PR other/57324
> * expmed.c (expand_smod_pow2): Use an unsigned -1 for lshift.
> * fold-const.c (fold_unary_loc): Likewise.
> * double-int.c (rshift_double, lshift_double): Likewise.
> * cse.c (cse_insn): Likewise.
> * tree.c (integer_pow2p, tree_log2, tree_floor_log2): Likewise.
> * tree-ssa-structalias.c (UNKNOWN_OFFSET): Shift 1 instead of -1.
>
> --
> Marc Glisse
> Index: gcc/double-int.c
> ===================================================================
> --- gcc/double-int.c (revision 199256)
> +++ gcc/double-int.c (working copy)
> @@ -264,21 +264,22 @@ rshift_double (unsigned HOST_WIDE_INT l1
>
> if (count >= prec)
> {
> *hv = signmask;
> *lv = signmask;
> }
> else if ((prec - count) >= HOST_BITS_PER_DOUBLE_INT)
> ;
> else if ((prec - count) >= HOST_BITS_PER_WIDE_INT)
> {
> - *hv &= ~((HOST_WIDE_INT) (-1) << (prec - count -
> HOST_BITS_PER_WIDE_INT));
> + *hv &= ~((unsigned HOST_WIDE_INT) (-1)
> + << (prec - count - HOST_BITS_PER_WIDE_INT));
> *hv |= signmask << (prec - count - HOST_BITS_PER_WIDE_INT);
> }
> else
> {
> *hv = signmask;
> *lv &= ~((unsigned HOST_WIDE_INT) (-1) << (prec - count));
> *lv |= signmask << (prec - count);
> }
> }
>
> @@ -321,21 +322,21 @@ lshift_double (unsigned HOST_WIDE_INT l1
>
> signmask = -((prec > HOST_BITS_PER_WIDE_INT
> ? ((unsigned HOST_WIDE_INT) *hv
> >> (prec - HOST_BITS_PER_WIDE_INT - 1))
> : (*lv >> (prec - 1))) & 1);
>
> if (prec >= HOST_BITS_PER_DOUBLE_INT)
> ;
> else if (prec >= HOST_BITS_PER_WIDE_INT)
> {
> - *hv &= ~((HOST_WIDE_INT) (-1) << (prec - HOST_BITS_PER_WIDE_INT));
> + *hv &= ~((unsigned HOST_WIDE_INT) -1 << (prec -
> HOST_BITS_PER_WIDE_INT));
> *hv |= signmask << (prec - HOST_BITS_PER_WIDE_INT);
> }
> else
> {
> *hv = signmask;
> *lv &= ~((unsigned HOST_WIDE_INT) (-1) << prec);
> *lv |= signmask << prec;
> }
> }
>
> Index: gcc/tree-ssa-structalias.c
> ===================================================================
> --- gcc/tree-ssa-structalias.c (revision 199256)
> +++ gcc/tree-ssa-structalias.c (working copy)
> @@ -475,21 +475,21 @@ struct constraint_expr
>
> /* Offset, in bits, of this constraint from the beginning of
> variables it ends up referring to.
>
> IOW, in a deref constraint, we would deref, get the result set,
> then add OFFSET to each member. */
> HOST_WIDE_INT offset;
> };
>
> /* Use 0x8000... as special unknown offset. */
> -#define UNKNOWN_OFFSET ((HOST_WIDE_INT)-1 << (HOST_BITS_PER_WIDE_INT-1))
> +#define UNKNOWN_OFFSET ((HOST_WIDE_INT) 1 << (HOST_BITS_PER_WIDE_INT-1))
>
> typedef struct constraint_expr ce_s;
> static void get_constraint_for_1 (tree, vec<ce_s> *, bool, bool);
> static void get_constraint_for (tree, vec<ce_s> *);
> static void get_constraint_for_rhs (tree, vec<ce_s> *);
> static void do_deref (vec<ce_s> *);
>
> /* Our set constraints are made up of two constraint expressions, one
> LHS, and one RHS.
>
> Index: gcc/cse.c
> ===================================================================
> --- gcc/cse.c (revision 199256)
> +++ gcc/cse.c (working copy)
> @@ -5374,21 +5374,21 @@ cse_insn (rtx insn)
> may not equal what was stored, due to truncation. */
>
> if (GET_CODE (SET_DEST (sets[i].rtl)) == ZERO_EXTRACT)
> {
> rtx width = XEXP (SET_DEST (sets[i].rtl), 1);
>
> if (src_const != 0 && CONST_INT_P (src_const)
> && CONST_INT_P (width)
> && INTVAL (width) < HOST_BITS_PER_WIDE_INT
> && ! (INTVAL (src_const)
> - & ((HOST_WIDE_INT) (-1) << INTVAL (width))))
> + & ((unsigned HOST_WIDE_INT) (-1) << INTVAL (width))))
> /* Exception: if the value is constant,
> and it won't be truncated, record it. */
> ;
> else
> {
> /* This is chosen so that the destination will be invalidated
> but no new value will be recorded.
> We must invalidate because sometimes constant
> values can be recorded for bitfields. */
> sets[i].src_elt = 0;
> Index: gcc/expmed.c
> ===================================================================
> --- gcc/expmed.c (revision 199256)
> +++ gcc/expmed.c (working copy)
> @@ -3688,39 +3688,39 @@ expand_smod_pow2 (enum machine_mode mode
> }
>
> /* Mask contains the mode's signbit and the significant bits of the
> modulus. By including the signbit in the operation, many targets
> can avoid an explicit compare operation in the following comparison
> against zero. */
>
> masklow = ((HOST_WIDE_INT) 1 << logd) - 1;
> if (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT)
> {
> - masklow |= (HOST_WIDE_INT) -1 << (GET_MODE_BITSIZE (mode) - 1);
> + masklow |= (unsigned HOST_WIDE_INT) -1 << (GET_MODE_BITSIZE (mode) -
> 1);
> maskhigh = -1;
> }
> else
> - maskhigh = (HOST_WIDE_INT) -1
> + maskhigh = (unsigned HOST_WIDE_INT) -1
> << (GET_MODE_BITSIZE (mode) - HOST_BITS_PER_WIDE_INT - 1);
>
> temp = expand_binop (mode, and_optab, op0,
> immed_double_const (masklow, maskhigh, mode),
> result, 1, OPTAB_LIB_WIDEN);
> if (temp != result)
> emit_move_insn (result, temp);
>
> label = gen_label_rtx ();
> do_cmp_and_jump (result, const0_rtx, GE, mode, label);
>
> temp = expand_binop (mode, sub_optab, result, const1_rtx, result,
> 0, OPTAB_LIB_WIDEN);
> - masklow = (HOST_WIDE_INT) -1 << logd;
> + masklow = (unsigned HOST_WIDE_INT) -1 << logd;
> maskhigh = -1;
> temp = expand_binop (mode, ior_optab, temp,
> immed_double_const (masklow, maskhigh, mode),
> result, 1, OPTAB_LIB_WIDEN);
> temp = expand_binop (mode, add_optab, temp, const1_rtx, result,
> 0, OPTAB_LIB_WIDEN);
> if (temp != result)
> emit_move_insn (result, temp);
> emit_label (label);
> return result;
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c (revision 199256)
> +++ gcc/tree.c (working copy)
> @@ -1935,26 +1935,26 @@ integer_pow2p (const_tree expr)
> prec = TYPE_PRECISION (TREE_TYPE (expr));
> high = TREE_INT_CST_HIGH (expr);
> low = TREE_INT_CST_LOW (expr);
>
> /* First clear all bits that are beyond the type's precision in case
> we've been sign extended. */
>
> if (prec == HOST_BITS_PER_DOUBLE_INT)
> ;
> else if (prec > HOST_BITS_PER_WIDE_INT)
> - high &= ~((HOST_WIDE_INT) (-1) << (prec - HOST_BITS_PER_WIDE_INT));
> + high &= ~((unsigned HOST_WIDE_INT) (-1) << (prec -
> HOST_BITS_PER_WIDE_INT));
> else
> {
> high = 0;
> if (prec < HOST_BITS_PER_WIDE_INT)
> - low &= ~((HOST_WIDE_INT) (-1) << prec);
> + low &= ~((unsigned HOST_WIDE_INT) (-1) << prec);
> }
>
> if (high == 0 && low == 0)
> return 0;
>
> return ((high == 0 && (low & (low - 1)) == 0)
> || (low == 0 && (high & (high - 1)) == 0));
> }
>
> /* Return 1 if EXPR is an integer constant other than zero or a
> @@ -1999,26 +1999,26 @@ tree_log2 (const_tree expr)
> prec = TYPE_PRECISION (TREE_TYPE (expr));
> high = TREE_INT_CST_HIGH (expr);
> low = TREE_INT_CST_LOW (expr);
>
> /* First clear all bits that are beyond the type's precision in case
> we've been sign extended. */
>
> if (prec == HOST_BITS_PER_DOUBLE_INT)
> ;
> else if (prec > HOST_BITS_PER_WIDE_INT)
> - high &= ~((HOST_WIDE_INT) (-1) << (prec - HOST_BITS_PER_WIDE_INT));
> + high &= ~((unsigned HOST_WIDE_INT) (-1) << (prec -
> HOST_BITS_PER_WIDE_INT));
> else
> {
> high = 0;
> if (prec < HOST_BITS_PER_WIDE_INT)
> - low &= ~((HOST_WIDE_INT) (-1) << prec);
> + low &= ~((unsigned HOST_WIDE_INT) (-1) << prec);
> }
>
> return (high != 0 ? HOST_BITS_PER_WIDE_INT + exact_log2 (high)
> : exact_log2 (low));
> }
>
> /* Similar, but return the largest integer Y such that 2 ** Y is less
> than or equal to EXPR. */
>
> int
> @@ -2036,26 +2036,26 @@ tree_floor_log2 (const_tree expr)
> high = TREE_INT_CST_HIGH (expr);
> low = TREE_INT_CST_LOW (expr);
>
> /* First clear all bits that are beyond the type's precision in case
> we've been sign extended. Ignore if type's precision hasn't been set
> since what we are doing is setting it. */
>
> if (prec == HOST_BITS_PER_DOUBLE_INT || prec == 0)
> ;
> else if (prec > HOST_BITS_PER_WIDE_INT)
> - high &= ~((HOST_WIDE_INT) (-1) << (prec - HOST_BITS_PER_WIDE_INT));
> + high &= ~((unsigned HOST_WIDE_INT) (-1) << (prec -
> HOST_BITS_PER_WIDE_INT));
> else
> {
> high = 0;
> if (prec < HOST_BITS_PER_WIDE_INT)
> - low &= ~((HOST_WIDE_INT) (-1) << prec);
> + low &= ~((unsigned HOST_WIDE_INT) (-1) << prec);
> }
>
> return (high != 0 ? HOST_BITS_PER_WIDE_INT + floor_log2 (high)
> : floor_log2 (low));
> }
>
> /* Return 1 if EXPR is the real constant zero. Trailing zeroes matter for
> decimal float constants, so don't return 1 for them. */
>
> int
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c (revision 199256)
> +++ gcc/fold-const.c (working copy)
> @@ -8049,21 +8049,21 @@ fold_unary_loc (location_t loc, enum tre
> || (TYPE_PRECISION (type)
> <= TYPE_PRECISION (TREE_TYPE (and_expr))))
> change = 1;
> else if (TYPE_PRECISION (TREE_TYPE (and1))
> <= HOST_BITS_PER_WIDE_INT
> && host_integerp (and1, 1))
> {
> unsigned HOST_WIDE_INT cst;
>
> cst = tree_low_cst (and1, 1);
> - cst &= (HOST_WIDE_INT) -1
> + cst &= (unsigned HOST_WIDE_INT) -1
> << (TYPE_PRECISION (TREE_TYPE (and1)) - 1);
> change = (cst == 0);
> #ifdef LOAD_EXTEND_OP
> if (change
> && !flag_syntax_only
> && (LOAD_EXTEND_OP (TYPE_MODE (TREE_TYPE (and0)))
> == ZERO_EXTEND))
> {
> tree uns = unsigned_type_for (TREE_TYPE (and0));
> and0 = fold_convert_loc (loc, uns, and0);
>
