Re: [wide-int] Remove tree_fits_hwi_p and tree_to_hwi

[Richard Sandiford <rdsandiford at googlemail dot com>]

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> On 11/17/2013 05:29 AM, Richard Sandiford wrote:
>> AIUI the two-argument tree_fits_hwi_p and tree_to_hwi were replacements
>> for host_integerp and tree_low_cst with variable "pos" arguments.
>> I removed those uses from trunk this week, and Mike's merge has
>> brought that into branch.
> i think that i am a little uncomfortable with this.  last night when i 
> was testing some stuff, i noticed a syntax error that i assume was a 
> merge problem in dwarf2out.c, but i did not run down where it came from.
>
> The code was at line 17499 and it used to be:
>
>        if (tree_fits_hwi_p (value)
>            && (simple_type_size_in_bits (TREE_TYPE (value))
>            <= HOST_BITS_PER_WIDE_INT || tree_fits_shwi_p (value)))
>
> someone (I assume you) had removed the tree_fits_hwi_p && and had left 
> the last parentheses.

Well, I removed the call from trunk and Mike left the extra parenthesis
when dealing with the resulting merge conflict.

As per the off-list message, I committed a patch to get the branch
building again this morning.

> However the code on the trunk is
>
>        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)))
>
> so i do not see how you have matched this functionality by removing that 
> call.

Are you sure you're looking at an up-to-date trunk?  It was removed by:

    http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01673.html

which was committed on Friday as r204846.  There was also a change
to the C frontend committed as r204847.  Those were the changes I was
talking about in the quote above.  So current trunk doesn't have any
calls to host_integerp or tree_low_cst with variable "pos" arguments.

Thanks,
Richard

>> The only remaining use is in a branch-local change to the way that
>> match_case_to_enum_1 prints constants.  The old code was:
>>
>>    /* ??? Not working too hard to print the double-word value.
>>       Should perhaps be done with %lwd in the diagnostic routines?  */
>>    if (TREE_INT_CST_HIGH (key) == 0)
>>      snprintf (buf, sizeof (buf), HOST_WIDE_INT_PRINT_UNSIGNED,
>> 	      TREE_INT_CST_LOW (key));
>>    else if (!TYPE_UNSIGNED (type)
>> 	   && TREE_INT_CST_HIGH (key) == -1
>> 	   && TREE_INT_CST_LOW (key) != 0)
>>      snprintf (buf, sizeof (buf), "-" HOST_WIDE_INT_PRINT_UNSIGNED,
>> 	      -TREE_INT_CST_LOW (key));
>>    else
>>      snprintf (buf, sizeof (buf), HOST_WIDE_INT_PRINT_DOUBLE_HEX,
>> 	      (unsigned HOST_WIDE_INT) TREE_INT_CST_HIGH (key),
>> 	      (unsigned HOST_WIDE_INT) TREE_INT_CST_LOW (key));
>>
>> The first arm prints KEY as an unsigned HWI if the "infinite precision"
>> value of KEY fits in an unsigned HWI (regardless of whether the type
>> is signed or not, e.g. it could be a signed 128-bit type with a value
>> that happens to fit in an unsigned 64-bit HWI).  The second prints it
>> as a negative HWI if KEY is negative and fits.  The third arm is a hex
>> fallback.  But on branch we only print KEYs with signed types as decimal
>> if they fit in signed HWIs, which is different from the first arm on trunk.
>>
>> This patch restores the trunk choices and gets rid of the then-unused
>> functions.
>>
>> Also, the single-argument tree_fits_hwi_p was used only in one place,
>> sdbout.c.  On trunk it's a "host_integerp (..., 0)" call, so that
>> translates to tree_fits_shwi_p on branch.
>>
>> Tested on x86_64-linux-gnu.  OK to install?
>>
>> Thanks,
>> Richard
>>
>>
>> Index: gcc/c-family/c-common.c
>> ===================================================================
>> --- gcc/c-family/c-common.c	2013-11-16 22:21:20.716272495 +0000
>> +++ gcc/c-family/c-common.c	2013-11-16 22:36:56.575137937 +0000
>> @@ -6056,8 +6056,10 @@ match_case_to_enum_1 (tree key, tree typ
>>   {
>>     char buf[WIDE_INT_PRINT_BUFFER_SIZE];
>>   
>> -  if (tree_fits_hwi_p (key, TYPE_SIGN (type)))
>> -    print_dec (key, buf, TYPE_SIGN (type));
>> +  if (tree_fits_uhwi_p (key))
>> +    print_dec (key, buf, UNSIGNED);
>> +  else if (tree_fits_shwi_p (key))
>> +    print_dec (key, buf, SIGNED);
>>     else
>>       print_hex (key, buf);
>>   
>> Index: gcc/doc/generic.texi
>> ===================================================================
>> --- gcc/doc/generic.texi	2013-11-16 22:21:23.034289829 +0000
>> +++ gcc/doc/generic.texi	2013-11-16 22:40:47.479832766 +0000
>> @@ -1024,10 +1024,8 @@ As this example indicates, the operands
>>   @tindex INTEGER_CST
>>   @tindex tree_fits_uhwi_p
>>   @tindex tree_fits_shwi_p
>> -@tindex tree_fits_hwi_p
>>   @tindex tree_to_uhwi
>>   @tindex tree_to_shwi
>> -@tindex tree_to_hwi
>>   @tindex REAL_CST
>>   @tindex FIXED_CST
>>   @tindex COMPLEX_CST
>> @@ -1050,16 +1048,11 @@ represented in an array of HOST_WIDE_INT
>>   in the array to represent the value without taking extra elements for
>>   redundant 0s or -1.
>>   
>> -The functions @code{tree_fits_uhwi_p}, @code{tree_fits_shwi_p}, and
>> -@code{tree_fits_hwi_p} can be used to tell if the value is small
>> -enough to fit in a HOST_WIDE_INT, as either a signed value, an unsiged
>> -value or a value whose sign is given as a parameter.  The value can
>> -then be extracted using the @code{tree_to_uhwi}, @code{tree_to_shwi},
>> -or @code{tree_to_hwi}.  The @code{tree_to_hwi} comes in both checked
>> -and unchecked flavors.  However, when the value is used in a context
>> -where it may represent a value that is larger than can be represented
>> -in HOST_BITS_PER_WIDE_INT bits, the wide_int class should be used to
>> -manipulate the constant.
>> +The functions @code{tree_fits_shwi_p} and @code{tree_fits_uhwi_p}
>> +can be used to tell if the value is small enough to fit in a
>> +signed HOST_WIDE_INT or an unsigned HOST_WIDE_INT respectively.
>> +The value can then be extracted using @code{tree_to_shwi} and
>> +@code{tree_to_uhwi}.
>>   
>>   @item REAL_CST
>>   
>> Index: gcc/sdbout.c
>> ===================================================================
>> --- gcc/sdbout.c	2013-10-22 10:15:14.050507121 +0100
>> +++ gcc/sdbout.c	2013-11-16 22:37:51.168539468 +0000
>> @@ -1152,7 +1152,7 @@ sdbout_one_type (tree type)
>>   	        if (TREE_CODE (value) == CONST_DECL)
>>   	          value = DECL_INITIAL (value);
>>   
>> -	        if (tree_fits_hwi_p (value))
>> +	        if (tree_fits_shwi_p (value))
>>   		  {
>>   		    PUT_SDB_DEF (IDENTIFIER_POINTER (TREE_PURPOSE (tem)));
>>   		    PUT_SDB_INT_VAL (tree_to_shwi (value));
>> Index: gcc/tree.h
>> ===================================================================
>> --- gcc/tree.h	2013-11-16 22:21:25.473308066 +0000
>> +++ gcc/tree.h	2013-11-16 22:42:05.148400937 +0000
>> @@ -3235,37 +3235,6 @@ tree_fits_shwi_p (const_tree cst)
>>     return false;
>>   }
>>   
>> -/* Return true if T is an INTEGER_CST that can be manipulated
>> -   efficiently on the host.  If SIGN is SIGNED, the value can be
>> -   represented in a single HOST_WIDE_INT.  If SIGN is UNSIGNED, the
>> -   value must be non-negative and can be represented in a single
>> -   unsigned HOST_WIDE_INT.  */
>> -
>> -static inline bool
>> -tree_fits_hwi_p (const_tree cst, signop sign)
>> -{
>> -  return sign ? tree_fits_uhwi_p (cst) : tree_fits_shwi_p (cst);
>> -}
>> -
>> -/* Return true if T is an INTEGER_CST that can be manipulated
>> -   efficiently on the host.  If the sign of CST is SIGNED, the value
>> -   can be represented in a single HOST_WIDE_INT.  If the sign of CST
>> -   is UNSIGNED, the value must be non-negative and can be represented
>> -   in a single unsigned HOST_WIDE_INT.  */
>> -
>> -static inline bool
>> -tree_fits_hwi_p (const_tree cst)
>> -{
>> -  if (cst == NULL_TREE)
>> -    return false;
>> -
>> -  if (TREE_CODE (cst) != INTEGER_CST)
>> -    return false;
>> -
>> -  return TYPE_UNSIGNED (TREE_TYPE (cst))
>> -    ? tree_fits_uhwi_p (cst) : tree_fits_shwi_p (cst);
>> -}
>> -
>>   /* Return the unsigned HOST_WIDE_INT least significant bits of CST.
>>      If checking is enabled, this ices if the value does not fit.  */
>>   
>> @@ -3298,19 +3267,6 @@ tree_to_hwi (const_tree cst)
>>     return TREE_INT_CST_ELT (cst, 0);
>>   }
>>   
>> -/* Return the HOST_WIDE_INT least significant bits of CST.  The sign
>> -   of the checking is based on SIGNOP. */
>> -
>> -static inline HOST_WIDE_INT
>> -tree_to_hwi (const_tree cst, signop sgn)
>> -{
>> -  if (sgn == SIGNED)
>> -    return tree_to_shwi (cst);
>> -  else
>> -    return tree_to_uhwi (cst);
>> -}
>> -
>> -
>>   /* Compute the number of operands in an expression node NODE.  For
>>      tcc_vl_exp nodes like CALL_EXPRs, this is stored in the node itself,
>>      otherwise it is looked up from the node's code.  */
>>
>>