[v2] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value

The original length 18 is not enough for HOST_WIDE_INT printing, need
use 20 instead of.

Also need additional bytes for printing related prefix and suffix, and
define the related macro in "hwint.h".

It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.


2014-11-23  Chen Gang <gang.chen.5i5j@gmail.com> 

	* c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let
	buffer enough to print host wide integer value.
---
 gcc/c-family/c-cppbuiltin.c | 30 +++++++++++++++++++++++-------
 gcc/hwint.h                 |  6 ++++++
 2 files changed, 29 insertions(+), 7 deletions(-)

On Sun, 23 Nov 2014, Chen Gang wrote:

> +  gcc_assert (wi::fits_to_tree_p(value, integer_type_node));

Watch formatting: space before '(' in the wi::fits_to_tree_p call.  
Applies elsewhere in this patch as well.

When making such an interface change, (a) you should update the comment on 
builtin_define_with_int_value to explain the new interface, and (b) you 
should check existing callers to make sure their values are indeed in 
range, and describe the check you did.

In fact, -fabi-version=0 results in __GXX_ABI_VERSION being defined to 
999999 using builtin_define_with_int_value.  That's out of range of int on 
targets with 16-bit int.  So that indicates against requiring the value to 
be within range of int.  It might however be OK to require the value to be 
within range of target long.

> +  if (value >= 0)
> +    {
> +      sprintf (buf, "%s="HOST_WIDE_INT_PRINT_DEC"%s",
> +	       macro, value,
> +	       value <= HOST_INT_MAX
> +	       ? ""
> +	       : value <= HOST_LONG_MAX
> +		 ? "L" : "LL");

Limits on the host's int and long are completely irrelevant here.  The 
question is the target's int and long, not the host's - and consistency 
indicates checking with wi::fits_to_tree_p (value, integer_type_node) if 
the assertion checked with long_integer_type_node.

On 11/25/14 7:56, Joseph Myers wrote:
> On Sun, 23 Nov 2014, Chen Gang wrote:
> 
>> +  gcc_assert (wi::fits_to_tree_p(value, integer_type_node));
> 
> Watch formatting: space before '(' in the wi::fits_to_tree_p call.  
> Applies elsewhere in this patch as well.
> 

OK, thanks, I shall notice next.

> When making such an interface change, (a) you should update the comment on 
> builtin_define_with_int_value to explain the new interface, and (b) you 
> should check existing callers to make sure their values are indeed in 
> range, and describe the check you did.
> 
> In fact, -fabi-version=0 results in __GXX_ABI_VERSION being defined to 
> 999999 using builtin_define_with_int_value.  That's out of range of int on 
> targets with 16-bit int.  So that indicates against requiring the value to 
> be within range of int.  It might however be OK to require the value to be 
> within range of target long.
> 

For me, can let builtin_define_with_int_value() fit all kinds of integer
values, and the assert need be:

  gcc_assert (wi::fits_to_tree_p (value, char_type_node)
              || wi::fits_to_tree_p (value, short_integer_type_node)
              || wi::fits_to_tree_p (value, integer_type_node)
              || wi::fits_to_tree_p (value, long_integer_type_node)
              || wi::fits_to_tree_p (value, long_long_integer_type_node));

If it really can fit all kinds of integer values, for me, the related
comments of builtin_define_with_int_value() need not be changed.

>> +  if (value >= 0)
>> +    {
>> +      sprintf (buf, "%s="HOST_WIDE_INT_PRINT_DEC"%s",
>> +	       macro, value,
>> +	       value <= HOST_INT_MAX
>> +	       ? ""
>> +	       : value <= HOST_LONG_MAX
>> +		 ? "L" : "LL");
> 
> Limits on the host's int and long are completely irrelevant here.  The 
> question is the target's int and long, not the host's - and consistency 
> indicates checking with wi::fits_to_tree_p (value, integer_type_node) if 
> the assertion checked with long_integer_type_node.
> 

OK, thanks. And for me, the related sprintf() should be:

  sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
           macro,
           value < 0 ? "(" : "",
           value,
           wi::fits_to_tree_p (value, char_type_node)
             || wi::fits_to_tree_p (value, short_integer_type_node)
             || wi::fits_to_tree_p (value, integer_type_node)
           ? "" 
           : wi::fits_to_tree_p (value, long_integer_type_node)
             ? "L" : "LL",
           value < 0 ? ")" : ""); 

Thanks.