Re: Remove host_integerp tests with variable signedness

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

On Thu, Nov 14, 2013 at 10:04 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Apart from the case in the C front end, there are 4 calls to host_integerp
> with a variable "pos" argument.  These "pos" arguments are all taken from
> TYPE_UNSIGNED.  In the dwarf2out.c case we go on to require:
>
>   simple_type_size_in_bits (TREE_TYPE (value)) <= HOST_BITS_PER_WIDE_INT
>   || host_integerp (value, 0)
>
> The host_integerp (value, 0) makes the first host_integerp trivially
> redundant for !TYPE_UNSIGNED.  Checking that the precision is
> <= HOST_BITS_PER_WIDE_INT makes the first host_integerp redundant
> for TYPE_UNSIGNED too, since all unsigned types of those precisions
> will fit in an unsigned HWI.  We already know that we're dealing with
> an INTEGER_CST, so there's no need for a code check either.
>
> vect_recog_divmod_pattern is similar in the sense that we already know
> that we have an INTEGER_CST and that we specifically check for precisions
> <= HOST_BITS_PER_WIDE_INT.
>
> In the other two cases we don't know whether we're dealing with an
> INTEGER_CST but we do check for precisions <= HOST_BITS_PER_WIDE_INT.
> So these host_integerps reduce to code tests.  (The precision check
> for expand_vector_divmod doesn't show up in the context but is at
> the top of the function:
>
>   if (prec > HOST_BITS_PER_WIDE_INT)
>     return NULL_TREE;
> )
>
> I also replaced the associated tree_low_csts with TREE_INT_CST_LOWs.
>
> Tested on x86_64-linux-gnu.  OK to install?

Note that host_integerp "conveniently" also makes sure the argument
is not NULL and of INTEGER_CST kind, so without more context ....

> Thanks,
> Richard
>
>
> gcc/
>         * dwarf2out.c (gen_enumeration_type_die): Remove unnecessary
>         host_integerp test.
>         * tree-vect-patterns.c (vect_recog_divmod_pattern): Likewise.
>         Use TREE_INT_CST_LOW rather than tree_low_cst when reading the
>         constant.
>         * fold-const.c (fold_binary_loc): Replace a host_integerp/tree_low_cst
>         pair with a TREE_CODE test and TREE_INT_CST_LOW.
>         * tree-vect-generic.c (expand_vector_divmod): Likewise.
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c     2013-11-14 20:21:27.183058648 +0000
> +++ gcc/dwarf2out.c     2013-11-14 20:22:18.128829681 +0000
> @@ -17321,9 +17321,8 @@ gen_enumeration_type_die (tree type, dw_
>           if (TREE_CODE (value) == CONST_DECL)
>             value = DECL_INITIAL (value);
>
> -         if (host_integerp (value, TYPE_UNSIGNED (TREE_TYPE (value)))
> -             && (simple_type_size_in_bits (TREE_TYPE (value))
> -                 <= HOST_BITS_PER_WIDE_INT || host_integerp (value, 0)))
> +         if (simple_type_size_in_bits (TREE_TYPE (value))
> +             <= HOST_BITS_PER_WIDE_INT || host_integerp (value, 0))

... this isn't a 1:1 tranform

>             /* DWARF2 does not provide a way of indicating whether or
>                not enumeration constants are signed or unsigned.  GDB
>                always assumes the values are signed, so we output all
> Index: gcc/tree-vect-patterns.c
> ===================================================================
> --- gcc/tree-vect-patterns.c    2013-11-14 20:21:27.183058648 +0000
> +++ gcc/tree-vect-patterns.c    2013-11-14 20:22:18.129829676 +0000
> @@ -2064,9 +2064,8 @@ vect_recog_divmod_pattern (vec<gimple> *
>        return pattern_stmt;
>      }
>
> -  if (!host_integerp (oprnd1, TYPE_UNSIGNED (itype))
> -      || integer_zerop (oprnd1)
> -      || prec > HOST_BITS_PER_WIDE_INT)
> +  if (prec > HOST_BITS_PER_WIDE_INT
> +      || integer_zerop (oprnd1))

likewise.

>      return NULL;
>
>    if (!can_mult_highpart_p (TYPE_MODE (vectype), TYPE_UNSIGNED (itype)))
> @@ -2078,8 +2077,8 @@ vect_recog_divmod_pattern (vec<gimple> *
>      {
>        unsigned HOST_WIDE_INT mh, ml;
>        int pre_shift, post_shift;
> -      unsigned HOST_WIDE_INT d = tree_low_cst (oprnd1, 1)
> -                                & GET_MODE_MASK (TYPE_MODE (itype));

This ensures the value is positive ...

> +      unsigned HOST_WIDE_INT d = (TREE_INT_CST_LOW (oprnd1)
> +                                 & GET_MODE_MASK (TYPE_MODE (itype)));
>        tree t1, t2, t3, t4;
>
>        if (d >= ((unsigned HOST_WIDE_INT) 1 << (prec - 1)))
> @@ -2195,7 +2194,7 @@ vect_recog_divmod_pattern (vec<gimple> *
>      {
>        unsigned HOST_WIDE_INT ml;
>        int post_shift;
> -      HOST_WIDE_INT d = tree_low_cst (oprnd1, 0);
> +      HOST_WIDE_INT d = TREE_INT_CST_LOW (oprnd1);

While this also allows negatives.

>        unsigned HOST_WIDE_INT abs_d;
>        bool add = false;
>        tree t1, t2, t3, t4;
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c    2013-11-14 20:21:27.183058648 +0000
> +++ gcc/fold-const.c    2013-11-14 20:22:18.124829699 +0000
> @@ -12032,16 +12032,15 @@ fold_binary_loc (location_t loc,
>          if the new mask might be further optimized.  */
>        if ((TREE_CODE (arg0) == LSHIFT_EXPR
>            || TREE_CODE (arg0) == RSHIFT_EXPR)
> -         && host_integerp (TREE_OPERAND (arg0, 1), 1)
> -         && host_integerp (arg1, TYPE_UNSIGNED (TREE_TYPE (arg1)))
> -         && tree_low_cst (TREE_OPERAND (arg0, 1), 1)
> -            < TYPE_PRECISION (TREE_TYPE (arg0))
>           && TYPE_PRECISION (TREE_TYPE (arg0)) <= HOST_BITS_PER_WIDE_INT
> -         && tree_low_cst (TREE_OPERAND (arg0, 1), 1) > 0)
> +         && TREE_CODE (arg1) == INTEGER_CST
> +         && host_integerp (TREE_OPERAND (arg0, 1), 1)
> +         && tree_low_cst (TREE_OPERAND (arg0, 1), 1) > 0
> +         && (tree_low_cst (TREE_OPERAND (arg0, 1), 1)
> +             < TYPE_PRECISION (TREE_TYPE (arg0))))
>         {
>           unsigned int shiftc = tree_low_cst (TREE_OPERAND (arg0, 1), 1);
> -         unsigned HOST_WIDE_INT mask
> -           = tree_low_cst (arg1, TYPE_UNSIGNED (TREE_TYPE (arg1)));
> +         unsigned HOST_WIDE_INT mask = TREE_INT_CST_LOW (arg1);

Where's the size test on arg1 gone?  IMHO this whole code should
just use double-ints ...

>           unsigned HOST_WIDE_INT newmask, zerobits = 0;
>           tree shift_type = TREE_TYPE (arg0);
>
> Index: gcc/tree-vect-generic.c
> ===================================================================
> --- gcc/tree-vect-generic.c     2013-11-14 20:21:27.183058648 +0000
> +++ gcc/tree-vect-generic.c     2013-11-14 20:22:18.129829676 +0000
> @@ -431,7 +431,7 @@ expand_vector_divmod (gimple_stmt_iterat
>        tree cst = VECTOR_CST_ELT (op1, i);
>        unsigned HOST_WIDE_INT ml;
>
> -      if (!host_integerp (cst, unsignedp) || integer_zerop (cst))
> +      if (TREE_CODE (cst) != INTEGER_CST || integer_zerop (cst))
>         return NULL_TREE;
>        pre_shifts[i] = 0;
>        post_shifts[i] = 0;
> @@ -452,7 +452,7 @@ expand_vector_divmod (gimple_stmt_iterat
>        if (unsignedp)
>         {
>           unsigned HOST_WIDE_INT mh;
> -         unsigned HOST_WIDE_INT d = tree_low_cst (cst, 1) & mask;
> +         unsigned HOST_WIDE_INT d = TREE_INT_CST_LOW (cst) & mask;
>
>           if (d >= ((unsigned HOST_WIDE_INT) 1 << (prec - 1)))
>             /* FIXME: Can transform this into op0 >= op1 ? 1 : 0.  */
> @@ -522,7 +522,7 @@ expand_vector_divmod (gimple_stmt_iterat
>         }
>        else
>         {
> -         HOST_WIDE_INT d = tree_low_cst (cst, 0);
> +         HOST_WIDE_INT d = TREE_INT_CST_LOW (cst);
>           unsigned HOST_WIDE_INT abs_d;

That looks ok to me.

>           if (d == -1)