| Message ID | 5468B4F5.9020600@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, 16 Nov 2014, Chen Gang wrote: > The maximize 64-bits integer decimal string length excluding NUL is 20 ( > '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT. > > 2014-11-16 Chen Gang <gang.chen.5i5j@gmail.com> > > * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Use > 20 instead of 18 for the maximize 64-bits integer decimal > string length OK. (Though it's not a good idea to use builtin_define_with_int_value with large arguments, as it won't generate any suffixes for arguments outside the range of target int, and -9223372036854775808 would actually need to be expressed as (-9223372036854775807LL-1). I think it's OK that it doesn't parenthesize negative numbers when outputting them - that the only cases for which the lack of parentheses could affect the parse are invalid for other reasons - though parentheses around negative numbers output might be a good idea anyway to make it obvious the output is OK, and would accord with how some macros such as __*_MIN_EXP__ are output; those values should probably use builtin_define_with_int_value.)
On 11/19/14 0:44, Joseph Myers wrote: > On Sun, 16 Nov 2014, Chen Gang wrote: > >> The maximize 64-bits integer decimal string length excluding NUL is 20 ( >> '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT. >> >> 2014-11-16 Chen Gang <gang.chen.5i5j@gmail.com> >> >> * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Use >> 20 instead of 18 for the maximize 64-bits integer decimal >> string length > > OK. (Though it's not a good idea to use builtin_define_with_int_value > with large arguments, as it won't generate any suffixes for arguments > outside the range of target int, and -9223372036854775808 would actually > need to be expressed as (-9223372036854775807LL-1). I think it's OK that > it doesn't parenthesize negative numbers when outputting them - that the > only cases for which the lack of parentheses could affect the parse are > invalid for other reasons - though parentheses around negative numbers > output might be a good idea anyway to make it obvious the output is OK, > and would accord with how some macros such as __*_MIN_EXP__ are output; > those values should probably use builtin_define_with_int_value.) > OK, thanks, what you said sounds reasonable to me. We need '(' and ')' for negative members, and "LL" for the members which is larger than 32 bits. For me, for simplify thinking, can let all numbers have '(', ')' and "LL" (whether it is positive numbers or negative numbers, whether it is large numbers or small numbers), and one sprintf() is enough: sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC")LL", macro, value); And excuse me, I do not understand why use "(-9223372036854775807LL-1)" instead of "(-9223372036854775808LL)": compiler will report warning for it, but for me, it is more likely the compiler's own issue: bash-3.2# cat ./test.c #include <stdio.h> int main () { long long i = (-9223372036854775808LL); long long j = 0x8000000000000000LL; long long k = 0x7fffffffffffffffLL; printf("\ni: %lld, j: %lld, k: %lld\n", i, j, k); return 0; } bash-3.2# gcc -o test test.c test.c:5:19: warning: integer constant is larger than the largest signed integer type long long i = (-9223372036854775808LL); ^ 1 warning generated. bash-3.2# ./test i: -9223372036854775808, j: -9223372036854775808, k: 9223372036854775807 Thanks
On 11/19/14 11:03, Chen Gang wrote: > On 11/19/14 0:44, Joseph Myers wrote: >> On Sun, 16 Nov 2014, Chen Gang wrote: >> >>> The maximize 64-bits integer decimal string length excluding NUL is 20 ( >>> '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT. >>> >>> 2014-11-16 Chen Gang <gang.chen.5i5j@gmail.com> >>> >>> * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Use >>> 20 instead of 18 for the maximize 64-bits integer decimal >>> string length >> >> OK. (Though it's not a good idea to use builtin_define_with_int_value >> with large arguments, as it won't generate any suffixes for arguments >> outside the range of target int, and -9223372036854775808 would actually >> need to be expressed as (-9223372036854775807LL-1). I think it's OK that >> it doesn't parenthesize negative numbers when outputting them - that the >> only cases for which the lack of parentheses could affect the parse are >> invalid for other reasons - though parentheses around negative numbers >> output might be a good idea anyway to make it obvious the output is OK, >> and would accord with how some macros such as __*_MIN_EXP__ are output; >> those values should probably use builtin_define_with_int_value.) >> > > OK, thanks, what you said sounds reasonable to me. We need '(' and ')' > for negative members, and "LL" for the members which is larger than 32 > bits. > > For me, for simplify thinking, can let all numbers have '(', ')' and > "LL" (whether it is positive numbers or negative numbers, whether it is > large numbers or small numbers), and one sprintf() is enough: > > sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC")LL", macro, value); Oh, sorry, need be: sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC"LL)", macro, value); > > And excuse me, I do not understand why use "(-9223372036854775807LL-1)" > instead of "(-9223372036854775808LL)": compiler will report warning for > it, but for me, it is more likely the compiler's own issue: > > bash-3.2# cat ./test.c > #include <stdio.h> > > int main () > { > long long i = (-9223372036854775808LL); > long long j = 0x8000000000000000LL; > long long k = 0x7fffffffffffffffLL; > > printf("\ni: %lld, j: %lld, k: %lld\n", i, j, k); > > return 0; > } > bash-3.2# gcc -o test test.c > test.c:5:19: warning: integer constant is larger than the largest signed integer type > long long i = (-9223372036854775808LL); > ^ > 1 warning generated. > bash-3.2# ./test > > i: -9223372036854775808, j: -9223372036854775808, k: 9223372036854775807 > > > > Thanks >
On Wed, 19 Nov 2014, Chen Gang wrote: > OK, thanks, what you said sounds reasonable to me. We need '(' and ')' > for negative members, and "LL" for the members which is larger than 32 > bits. I don't think LL is a good idea when not needed - quite possibly some of the macros are expected or required to have type int. Really I think it would be better to require that this function is only used for values that fit in target int (given appropriate checks on all the users to make sure they fit in with that), and put a corresponding assertion there. > And excuse me, I do not understand why use "(-9223372036854775807LL-1)" > instead of "(-9223372036854775808LL)": compiler will report warning for > it, but for me, it is more likely the compiler's own issue: Because - and 9223372036854775808LL are separate tokens, and the latter is not a valid long long value.
On 11/20/14 1:20, Joseph Myers wrote: > On Wed, 19 Nov 2014, Chen Gang wrote: > >> OK, thanks, what you said sounds reasonable to me. We need '(' and ')' >> for negative members, and "LL" for the members which is larger than 32 >> bits. > > I don't think LL is a good idea when not needed - quite possibly some of > the macros are expected or required to have type int. OK, thanks. I guess your meaning is: - If the value is small enough to be expressed by type 'int', we should not provide 'LL'. - If the value is positive number, we should not provide '(' and ')'. If what I guess is correct, I shall try to send patch v2 for it within this month. > Really I think it > would be better to require that this function is only used for values that > fit in target int (given appropriate checks on all the users to make sure > they fit in with that), and put a corresponding assertion there. > Excuse me, I am not quite familiar with "target int", could you provide more details for it? And if necessary, please help send patch v2 instead of me: - We need provide the best code for upstream. I guess, at present, you can, but I can't (I am not quite familiar with the related things). - I have planned to send another v2 patches for gcc, and also analyze some issues within gcc. I should finish them within this month (it seems, at present, I may delay again). - If can bear me, still let me send patch for it, I may send patch v2 in next month. Thanks. >> And excuse me, I do not understand why use "(-9223372036854775807LL-1)" >> instead of "(-9223372036854775808LL)": compiler will report warning for >> it, but for me, it is more likely the compiler's own issue: > > Because - and 9223372036854775808LL are separate tokens, and the latter is > not a valid long long value. > OK, thanks. But for the user (e.g. a normal programmer like me), will feel that: - (-9223372036854775808LL) is a valid value, since it prints correctly. - If it is reasonable to report warning for (-9223372036854775808LL), why do not also report warning for 0x8000000000000000LL. Thanks.
On Thu, 20 Nov 2014, Chen Gang wrote: > OK, thanks. I guess your meaning is: > > - If the value is small enough to be expressed by type 'int', we should > not provide 'LL'. Yes - and values not small enough should probably not be accepted by this function. > - If the value is positive number, we should not provide '(' and ')'. Yes. > > Really I think it > > would be better to require that this function is only used for values that > > fit in target int (given appropriate checks on all the users to make sure > > they fit in with that), and put a corresponding assertion there. > > > > Excuse me, I am not quite familiar with "target int", could you provide > more details for it? And if necessary, please help send patch v2 instead > of me: "target int" is the type int on the target, as opposed to on the host (with precision TYPE_PRECISION (integer_type_node)). I think you can use wi::fits_to_tree_p (n, integer_type_node) to test if n is in range for target int.
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c index 26fabc2..8e8cec4 100644 --- a/gcc/c-family/c-cppbuiltin.c +++ b/gcc/c-family/c-cppbuiltin.c @@ -1345,7 +1345,7 @@ builtin_define_with_int_value (const char *macro, HOST_WIDE_INT value) { char *buf; size_t mlen = strlen (macro); - size_t vlen = 18; + size_t vlen = 20; /* maximize value length: -9223372036854775808 */ size_t extra = 2; /* space for = and NUL. */ buf = (char *) alloca (mlen + vlen + extra);