Re: [PATCH] fix ILP32 bootstrap failures due to -Wformat-length
On 09/21/2016 06:28 PM, Martin Sebor wrote: PR target/77676 - powerpc64 and powerpc64le stage2 bootstrap fail gcc/testsuite/ChangeLog: 2016-09-21 Martin Sebor PR target/77676 * gcc.dg/tree-ssa/builtin-sprintf-2.c: Fix typo. * gcc.dg/tree-ssa/builtin-sprintf-3.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: New test. gcc/ChangeLog: 2016-09-21 Martin Sebor PR target/77676 * gimple-ssa-sprintf.c (target_int_min, target_int_max): Use HOST_BITS_PER_WIDE_INT, make a static local variable auto. (target_int_min): Correct computation. (format_integer): Use long long as the argument for the ll length modifier. (format_floating): Use target_int_max(). (get_string_length): Same. (format_string): Avoid setting the bounded flag for strings of unknown length. (try_substitute_return_value): Avoid setting range info when the result isn't bounded. * varasm.c (assemble_name): Increase buffer size. @@ -1471,6 +1473,10 @@ format_string (const conversion_spec &spec, tree arg) : 2 == warn_format_length ? 0 <= prec ? prec : 1 : HOST_WIDE_INT_MAX); + /* The result is bounded unless overriddden for a non-constant string s/overriddden/overridden/ OK with that change. jeff
Re: [PATCH] fix ILP32 bootstrap failures due to -Wformat-length
Jeff or Richard, As the two middle end maintainers familiar with my work, can one of you approve the updated patch? It's necessary to restore ILP32 bootstrap on a number of targets (pr77676). https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01477.html Thanks Martin On 09/21/2016 06:28 PM, Martin Sebor wrote: On 09/21/2016 05:09 PM, Jakub Jelinek wrote: On Wed, Sep 21, 2016 at 04:39:42PM -0600, Martin Sebor wrote: diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index dddb026..652d3fb 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -210,8 +210,8 @@ struct format_result static HOST_WIDE_INT target_int_min () { - static const unsigned HOST_WIDE_INT int_min -= 1LLU << (sizeof int_min * CHAR_BIT + const unsigned HOST_WIDE_INT int_min += 1LLU << (HOST_BITS_PER_WIDE_INT 1LLU should be really HOST_WIDE_INT_1U - TYPE_PRECISION (integer_type_node) + 1); Is the shift amount really what you mean? HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1 is usually 33 (or 17 or 9 in much rarer cases), so that is 0x2ULL. Don't you want instead = HOST_WIDE_INT_1U << (TYPE_PRECISION (ingeger_type_node) - 1); so that it will be 0x8000ULL? Great catch, thank you! That must have been a copy and paste mistake. @@ -221,8 +221,8 @@ target_int_min () static unsigned HOST_WIDE_INT target_int_max () { - static const unsigned HOST_WIDE_INT int_max -= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT + const unsigned HOST_WIDE_INT int_max += HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1); return int_max; } This is expectedly -1ULL >> 33, i.e. 0x7fffULL, which looks ok. Thanks for double checking that. There is a test case for this in the test suite for the optimization but there wasn't one for the INT_MIN case. I've added it in the attached patch. I'll beef up the return range testing some more before re-enabling the optimization. Martin
Re: [PATCH] fix ILP32 bootstrap failures due to -Wformat-length
On 09/21/2016 05:09 PM, Jakub Jelinek wrote: On Wed, Sep 21, 2016 at 04:39:42PM -0600, Martin Sebor wrote: diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index dddb026..652d3fb 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -210,8 +210,8 @@ struct format_result static HOST_WIDE_INT target_int_min () { - static const unsigned HOST_WIDE_INT int_min -= 1LLU << (sizeof int_min * CHAR_BIT + const unsigned HOST_WIDE_INT int_min += 1LLU << (HOST_BITS_PER_WIDE_INT 1LLU should be really HOST_WIDE_INT_1U - TYPE_PRECISION (integer_type_node) + 1); Is the shift amount really what you mean? HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1 is usually 33 (or 17 or 9 in much rarer cases), so that is 0x2ULL. Don't you want instead = HOST_WIDE_INT_1U << (TYPE_PRECISION (ingeger_type_node) - 1); so that it will be 0x8000ULL? Great catch, thank you! That must have been a copy and paste mistake. @@ -221,8 +221,8 @@ target_int_min () static unsigned HOST_WIDE_INT target_int_max () { - static const unsigned HOST_WIDE_INT int_max -= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT + const unsigned HOST_WIDE_INT int_max += HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1); return int_max; } This is expectedly -1ULL >> 33, i.e. 0x7fffULL, which looks ok. Thanks for double checking that. There is a test case for this in the test suite for the optimization but there wasn't one for the INT_MIN case. I've added it in the attached patch. I'll beef up the return range testing some more before re-enabling the optimization. Martin PR target/77676 - powerpc64 and powerpc64le stage2 bootstrap fail gcc/testsuite/ChangeLog: 2016-09-21 Martin Sebor PR target/77676 * gcc.dg/tree-ssa/builtin-sprintf-2.c: Fix typo. * gcc.dg/tree-ssa/builtin-sprintf-3.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: New test. gcc/ChangeLog: 2016-09-21 Martin Sebor PR target/77676 * gimple-ssa-sprintf.c (target_int_min, target_int_max): Use HOST_BITS_PER_WIDE_INT, make a static local variable auto. (target_int_min): Correct computation. (format_integer): Use long long as the argument for the ll length modifier. (format_floating): Use target_int_max(). (get_string_length): Same. (format_string): Avoid setting the bounded flag for strings of unknown length. (try_substitute_return_value): Avoid setting range info when the result isn't bounded. * varasm.c (assemble_name): Increase buffer size. diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index dddb026..d1d20ca 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -210,9 +210,9 @@ struct format_result static HOST_WIDE_INT target_int_min () { - static const unsigned HOST_WIDE_INT int_min -= 1LLU << (sizeof int_min * CHAR_BIT - - TYPE_PRECISION (integer_type_node) + 1); + const unsigned HOST_WIDE_INT int_min += HOST_WIDE_INT_M1U << (TYPE_PRECISION (integer_type_node) - 1); + return int_min; } @@ -221,8 +221,8 @@ target_int_min () static unsigned HOST_WIDE_INT target_int_max () { - static const unsigned HOST_WIDE_INT int_max -= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT + const unsigned HOST_WIDE_INT int_max += HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1); return int_max; } @@ -851,7 +851,9 @@ format_integer (const conversion_spec &spec, tree arg) case FMT_LEN_L: case FMT_LEN_ll: - dirtype = sign ? long_integer_type_node : long_unsigned_type_node; + dirtype = (sign + ? long_long_integer_type_node + : long_long_unsigned_type_node); break; case FMT_LEN_z: @@ -1366,7 +1368,7 @@ format_floating (const conversion_spec &spec, tree arg) *minmax[i] = mpfr_snprintf (NULL, 0, fmtstr, mpfrval); } - res.bounded = res.range.min < HOST_WIDE_INT_MAX; + res.bounded = res.range.min < target_int_max (); return res; } @@ -1420,7 +1422,7 @@ get_string_length (tree str) /* Set RES.BOUNDED to true if and only if all strings referenced by STR are known to be bounded (though not necessarily by their actual length but perhaps by their maximum possible length). */ - res.bounded = res.range.max < HOST_WIDE_INT_MAX; + res.bounded = res.range.max < target_int_max (); /* Set RES.CONSTANT to false even though that may be overly conservative in rare cases like: 'x ? a : b' where a and @@ -1471,6 +1473,10 @@ format_string (const conversion_spec &spec, tree arg) : 2 == warn_format_length ? 0 <= prec ? prec : 1 : HOST_WIDE_INT_MAX); + /* The result is bounded unless overriddden for a non-constant string + of an unknown length. */ + bool bounded = true; + if (spec.specifier == 'c') { if (spec.modifier == FMT_LEN_l) @@ -1550,16 +1556,17 @@
Re: [PATCH] fix ILP32 bootstrap failures due to -Wformat-length
On Wed, Sep 21, 2016 at 04:39:42PM -0600, Martin Sebor wrote: > diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c > index dddb026..652d3fb 100644 > --- a/gcc/gimple-ssa-sprintf.c > +++ b/gcc/gimple-ssa-sprintf.c > @@ -210,8 +210,8 @@ struct format_result > static HOST_WIDE_INT > target_int_min () > { > - static const unsigned HOST_WIDE_INT int_min > -= 1LLU << (sizeof int_min * CHAR_BIT > + const unsigned HOST_WIDE_INT int_min > += 1LLU << (HOST_BITS_PER_WIDE_INT 1LLU should be really HOST_WIDE_INT_1U > - TYPE_PRECISION (integer_type_node) + 1); Is the shift amount really what you mean? HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1 is usually 33 (or 17 or 9 in much rarer cases), so that is 0x2ULL. Don't you want instead = HOST_WIDE_INT_1U << (TYPE_PRECISION (ingeger_type_node) - 1); so that it will be 0x8000ULL? > @@ -221,8 +221,8 @@ target_int_min () > static unsigned HOST_WIDE_INT > target_int_max () > { > - static const unsigned HOST_WIDE_INT int_max > -= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT > + const unsigned HOST_WIDE_INT int_max > += HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT > - TYPE_PRECISION (integer_type_node) + 1); >return int_max; > } This is expectedly -1ULL >> 33, i.e. 0x7fffULL, which looks ok. Jakub
[PATCH] fix ILP32 bootstrap failures due to -Wformat-length
As mentioned in PR77676. the gimple-ssa-sprintf.c pass committed yesterday caused a number of bootstrap failures, including on ILP32 targets. All of them were due to differences between integer sizes between the host and the target. To unblock the powerpc64 bootstrap failure reported in the PR I temporarily disabled the -fprintf-return-value optimization this morning. The attached patch fixes the underlying problems. I've retested it on i386 (ILP32), x86_64, and powerpc64le, the latter with the optimization enabled (even though the patch doesn't enable it). I'd like to commit this patch first to unblock the ILP32 targets, then re-enable the optimization in a subsequent commit, and then look into fixing the outstanding minor issues in the test suite on ILP32 (there's just one failure in my i368 test run) and some of those pointed out by Jakub this morning in https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01436.html Thanks Martin PR target/77676 - powerpc64 and powerpc64le stage2 bootstrap fail gcc/testsuite/ChangeLog: 2016-09-21 Martin Sebor PR target/77676 * gcc.dg/tree-ssa/builtin-sprintf-2.c: Fix typo. * gcc.dg/tree-ssa/builtin-sprintf-3.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: New test. gcc/ChangeLog: 2016-09-21 Martin Sebor PR target/77676 * gimple-ssa-sprintf.c (target_int_min, target_int_max): Use HOST_BITS_PER_WIDE_INT, make a static local variable auto. (format_integer): Use long long as the argument for the ll length modifier. (format_floating): Use target_int_max(). (get_string_length): Same. (format_string): Avoid setting the bounded flag for strings of unknown length. (try_substitute_return_value): Avoid setting range info when the result isn't bounded. * varasm.c (assemble_name): Increase buffer size. diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index dddb026..652d3fb 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -210,8 +210,8 @@ struct format_result static HOST_WIDE_INT target_int_min () { - static const unsigned HOST_WIDE_INT int_min -= 1LLU << (sizeof int_min * CHAR_BIT + const unsigned HOST_WIDE_INT int_min += 1LLU << (HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1); return int_min; } @@ -221,8 +221,8 @@ target_int_min () static unsigned HOST_WIDE_INT target_int_max () { - static const unsigned HOST_WIDE_INT int_max -= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT + const unsigned HOST_WIDE_INT int_max += HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1); return int_max; } @@ -851,7 +851,9 @@ format_integer (const conversion_spec &spec, tree arg) case FMT_LEN_L: case FMT_LEN_ll: - dirtype = sign ? long_integer_type_node : long_unsigned_type_node; + dirtype = (sign + ? long_long_integer_type_node + : long_long_unsigned_type_node); break; case FMT_LEN_z: @@ -1366,7 +1368,7 @@ format_floating (const conversion_spec &spec, tree arg) *minmax[i] = mpfr_snprintf (NULL, 0, fmtstr, mpfrval); } - res.bounded = res.range.min < HOST_WIDE_INT_MAX; + res.bounded = res.range.min < target_int_max (); return res; } @@ -1420,7 +1422,7 @@ get_string_length (tree str) /* Set RES.BOUNDED to true if and only if all strings referenced by STR are known to be bounded (though not necessarily by their actual length but perhaps by their maximum possible length). */ - res.bounded = res.range.max < HOST_WIDE_INT_MAX; + res.bounded = res.range.max < target_int_max (); /* Set RES.CONSTANT to false even though that may be overly conservative in rare cases like: 'x ? a : b' where a and @@ -1471,6 +1473,10 @@ format_string (const conversion_spec &spec, tree arg) : 2 == warn_format_length ? 0 <= prec ? prec : 1 : HOST_WIDE_INT_MAX); + /* The result is bounded unless overriddden for a non-constant string + of an unknown length. */ + bool bounded = true; + if (spec.specifier == 'c') { if (spec.modifier == FMT_LEN_l) @@ -1550,16 +1556,17 @@ format_string (const conversion_spec &spec, tree arg) if (0 <= prec) { if ((unsigned)prec < slen.range.min - || slen.range.min >= HOST_WIDE_INT_MAX) + || slen.range.min >= target_int_max ()) slen.range.min = prec; if ((unsigned)prec < slen.range.max - || slen.range.max >= HOST_WIDE_INT_MAX) + || slen.range.max >= target_int_max ()) slen.range.max = prec; } - else if (slen.range.min >= HOST_WIDE_INT_MAX) + else if (slen.range.min >= target_int_max ()) { slen.range.min = max_bytes_for_unknown_str; slen.range.max = max_bytes_for_unknown_str; + bounded = false; } res.range = slen.range; @@ -1580,7 +1587,8 @@ format_string (const conversion_spec &spec, tree arg) res.range.max = width; /* Adjust BOUNDED if width happens to make them equal. */ - i