| Message ID | 87zjp6pue5.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com |
|---|---|
| State | New |
| Headers | show |
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)
Richard Biener <richard.guenther@gmail.com> writes: > 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 .... Right, which is why I was trying to list out the reasons why the INTEGER_CST check isn't needed in the first two cases. None of the cases can be null AFAIK. >> 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 The full context is: if (simple_type_size_in_bits (TREE_TYPE (value)) <= HOST_BITS_PER_WIDE_INT || host_integerp (value, 0)) /* 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 values as if they were signed. That means that enumeration constants with very large unsigned values will appear to have negative values in the debugger. TODO: the above comment is wrong, DWARF2 does provide DW_FORM_sdata/DW_FORM_udata to represent signed/unsigned data. This should be re-worked to use correct signed/unsigned int/double tags for all cases, instead of always treating as signed. */ add_AT_int (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value)); else /* Enumeration constants may be wider than HOST_WIDE_INT. Handle that here. */ add_AT_double (enum_die, DW_AT_const_value, TREE_INT_CST_HIGH (value), TREE_INT_CST_LOW (value)); We're dealing with a nonnull INTEGER_CST whatever happens. >> 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. This is midway through a function that has already checked: if (TREE_CODE (oprnd0) != SSA_NAME || TREE_CODE (oprnd1) != INTEGER_CST || TREE_CODE (itype) != INTEGER_TYPE || TYPE_PRECISION (itype) != GET_MODE_PRECISION (TYPE_MODE (itype))) return NULL; and where: prec = TYPE_PRECISION (itype); So even without the host_integerp test, the code in the patch only fails to exit if we have an INTEGER_CST whose precision is <= HOST_BITS_PER_WIDE_INT. In that case the host_integerp test is redundant, because a unsigned constant will fit in an unsigned HWI and a signed constant will fit in a signed HWI. >> @@ -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 ... This is in a: if (TYPE_UNSIGNED (itype)) { block that is only reached if the conditions above are met. >> + 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. This is in the else (!TYPE_UNSIGNED) arm. >> 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 ... Not sure which test you mean. The only arg1 test is the host_integerp one, which I'm trying to get rid of. The precision of arg0 is the same as the precision of arg1 in this context, so the same reasoning as above applies: once we've established the TYPE_PRECISION condition, we already know that an unsigned arg1 constant will fit in an unsigned HWI and that a signed arg1 constant will fit in a HWI. So just checking the code is enough. Thanks, Richard
On Fri, Nov 15, 2013 at 11:48 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > Richard Biener <richard.guenther@gmail.com> writes: >> 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 .... > > Right, which is why I was trying to list out the reasons why the > INTEGER_CST check isn't needed in the first two cases. None of the > cases can be null AFAIK. > >>> 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 > > The full context is: > > if (simple_type_size_in_bits (TREE_TYPE (value)) > <= HOST_BITS_PER_WIDE_INT || host_integerp (value, 0)) > /* 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 > values as if they were signed. That means that > enumeration constants with very large unsigned values > will appear to have negative values in the debugger. > > TODO: the above comment is wrong, DWARF2 does provide > DW_FORM_sdata/DW_FORM_udata to represent signed/unsigned data. > This should be re-worked to use correct signed/unsigned > int/double tags for all cases, instead of always treating as > signed. */ > add_AT_int (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value)); > else > /* Enumeration constants may be wider than HOST_WIDE_INT. Handle > that here. */ > add_AT_double (enum_die, DW_AT_const_value, > TREE_INT_CST_HIGH (value), TREE_INT_CST_LOW (value)); > > We're dealing with a nonnull INTEGER_CST whatever happens. > >>> 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. > > This is midway through a function that has already checked: > > if (TREE_CODE (oprnd0) != SSA_NAME > || TREE_CODE (oprnd1) != INTEGER_CST > || TREE_CODE (itype) != INTEGER_TYPE > || TYPE_PRECISION (itype) != GET_MODE_PRECISION (TYPE_MODE (itype))) > return NULL; > > and where: > > prec = TYPE_PRECISION (itype); > > So even without the host_integerp test, the code in the patch only fails > to exit if we have an INTEGER_CST whose precision is <= HOST_BITS_PER_WIDE_INT. > In that case the host_integerp test is redundant, because a unsigned constant > will fit in an unsigned HWI and a signed constant will fit in a signed HWI. > >>> @@ -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 ... > > This is in a: > > if (TYPE_UNSIGNED (itype)) > { > > block that is only reached if the conditions above are met. > >>> + 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. > > This is in the else (!TYPE_UNSIGNED) arm. > >>> 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 ... > > Not sure which test you mean. The only arg1 test is the host_integerp one, > which I'm trying to get rid of. The precision of arg0 is the same as > the precision of arg1 in this context, so the same reasoning as above > applies: once we've established the TYPE_PRECISION condition, we already > know that an unsigned arg1 constant will fit in an unsigned HWI and > that a signed arg1 constant will fit in a HWI. So just checking the > code is enough. Ah, indeed. The patch is ok - thanks for the explanations. Richard. > Thanks, > Richard
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)) /* 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)) 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)); + 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); 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); 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; if (d == -1)
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? 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.