On Thu, Nov 14, 2013 at 10:04 PM, Richard Sandiford
<rdsandif...@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)
