/scratch/tmp/gerald/gcc10-devel-work/gcc-10-20200223/gcc/value-prof.c: In function 'void dump_histogram_value(FILE*, histogram_value)': /scratch/tmp/gerald/gcc10-devel-work/gcc-10-20200223/gcc/value-prof.c:268:28 : error: format '%lld' expects argument of type 'long long int', but argument 3 has type 'int' [-Werror=format=] 268 | fprintf (dump_file, " all: %" PRId64 "%s, values: ", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 269 | abs ((int64_t) hist->hvalue.counters[0]), | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | int I believe this may be caused by 2020-02-18 Martin Liska <mliska@suse.cz> PR ipa/92924 * common.opt: Add -fprofile-reproducibility. * doc/invoke.texi: Document it. * value-prof.c (dump_histogram_value): Document and support behavior for counters[0] being a negative value. (get_nth_most_common_value): Handle negative counters[0] in respect to flag_profile_reproducible.
Wait I think this should have been std::abs rather than just abs :). Can you try that and see if that fixes the issue? on glibc targets abs is in the toplevel namespace for all types (I think) but on FreeBSD only the overload for int is in the toplevel namespace.
See https://sourceware.org/ml/gdb-patches/2020-02/msg01014.html also for the same reasoning but with gdb instead of GCC.
Gerald, I wanted to reproduce the error so that I could in turn verify the proposed fix of using std:abs would work. So I installed FreeBSD 11.3 (i686) in a VM. Bootstrap with GCC went fine. I couldn't get the failure building with clang either. How are you configuring & building? I checked both my gcc and clang object trees, and both end up with %lld (compiling with -save-temps, then examining the .ii output). So while I believe Andrew is right that this should be using std::abs, I'd still like to reproduce the problem and verify that using std::abs is sufficient to fix.
I think a preprocessed value-prof.ii from Gerald would help here...
Absolutely.
(In reply to Jeffrey A. Law from comment #3) > Gerald, I wanted to reproduce the error so that I could in turn verify the > proposed fix of using std:abs would work. I verified that the fix of using std::abs works in the situation where I had failures without. > So I installed FreeBSD 11.3 (i686) in a VM. Bootstrap with GCC went fine. > I couldn't get the failure building with clang either. I looked into this. Indeed my regular bootstraps do not fail, even without this patch. It is building the lang/gcc10-devel port, based on our weekly snapshots, that exhibited this behavior. > How are you configuring & building? Are you familiar with the FreeBSD ports system? The port resides in ports/lang/gcc10-devel and to reproduce the issue you need to remove ports/lang/gcc10-devel/files/patch-value-prof.c-buildfix that I added. I have not found yet why a "regular" bootstrap succeeds and fails in my environment when I build it as a port, but will try to dig into it this evening. > So while I believe Andrew is right that this should be using std::abs, I'd > still like to reproduce the problem and verify that using std::abs is > sufficient to fix. The fix is confirmed, and part of the FreeBSD ports tree now. However, I am not yet able to explain why it does occur in one case and not in the other.
So do you think you could attach preprocessed sources from both the working and failing builds so that we can look up at the differences?
The master branch has been updated by Jeff Law <law@gcc.gnu.org>: https://gcc.gnu.org/g:20a235a8b443a81ea0ec6a10f260b119f2193a69 commit r10-7032-g20a235a8b443a81ea0ec6a10f260b119f2193a69 Author: Jeff Law <law@redhat.com> Date: Wed Mar 4 16:25:11 2020 -0700 Fix format warning which showed up on FreeBSD 11.3. PR bootstrap/93962 * value-prof.c (dump_histogram_value): Use std::abs.
Fixed on the trunk, but kept open so that Gerald can attach the .ii file for further analysis if Jakub is interested.
There is another abs at line 746 which causes a bootstrap failure on Solaris 11: /homes/botcazou/gcc-head/src/gcc/value-prof.c: In function 'bool get_nth_most_common_value(gimple*, const char*, histogram_value, gcov_type*, gcov_type*, gcov_type*, unsigned int)': /homes/botcazou/gcc-head/src/gcc/value-prof.c:746:53: error: call of overloaded 'abs(gcov_type&)' is ambiguous gcov_type read_all = abs (hist->hvalue.counters[0]); ^ /homes/botcazou/gcc-head/src/gcc/value-prof.c:746:53: note: candidates are: In file included from /usr/include/stdlib.h:11:0, from /homes/botcazou/gcc-head/src/gcc/system.h:258, from /homes/botcazou/gcc-head/src/gcc/value-prof.c:21: /usr/include/iso/stdlib_iso.h:154:23: note: long int std::abs(long int) inline long abs(long _l) { return labs(_l); } ^ /usr/include/iso/stdlib_iso.h:108:12: note: int std::abs(int) extern int abs(int); ^ gmake[3]: *** [value-prof.o] Error 1
I think even the using of std::abs in the #c8 case isn't correct, because the std::abs (long long); overload has been only added in C++11 and we in GCC 10 still do support C++98 compilers. So I think we instead should use abs_hwi (or absu_hwi, depending on if the most negative value can appear or not) instead of std::abs or abs in value-prof.c. So e.g. --- gcc/value-prof.c 2020-03-05 07:58:02.693135980 +0100 +++ gcc/value-prof.c 2020-03-10 14:32:10.723649888 +0100 @@ -266,7 +266,7 @@ dump_histogram_value (FILE *dump_file, h if (hist->hvalue.counters) { fprintf (dump_file, " all: %" PRId64 "%s, values: ", - std::abs ((int64_t) hist->hvalue.counters[0]), + absu_hwi (hist->hvalue.counters[0]), hist->hvalue.counters[0] < 0 ? " (values missing)": ""); for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++) @@ -743,7 +743,7 @@ get_nth_most_common_value (gimple *stmt, *count = 0; *value = 0; - gcov_type read_all = abs (hist->hvalue.counters[0]); + gcov_type read_all = absu_hwi (hist->hvalue.counters[0]); gcov_type v = hist->hvalue.counters[2 * n + 1]; gcov_type c = hist->hvalue.counters[2 * n + 2]; or with s/absu/abs/.
Created attachment 48011 [details] Preprocessed value-prof.c for the failure case on FreeBSD 11/i386 (I've been struggling to create the preprocessed source files, but keep working on it. This is the first one - for the failure case.)
> So I think we instead should use abs_hwi (or absu_hwi, depending on if the > most negative value can appear or not) instead of std::abs or abs in > value-prof.c. No, the counter can never be INT64_MIN.
Ok, I'll test a patch with abs_hwi in both spots then. There will be an assertion in there that it is not INT64_MIN.
commit r10-7122-g60342fdbfb0630243d2b85d2ca45204ded990b17 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Mar 11 09:34:59 2020 +0100 value-prof: Fix abs uses in value-prof.c [PR93962] Jeff has recently fixed dump_histogram_value to use std::abs instead of abs, because on FreeBSD apparently the ::abs isn't overloaded and only has int abs (int); Seems on Solaris /usr/include/iso/stdlib_iso.h abs has: int abs (int); long abs (long); overloads but already not long long abs (long long); and there is another abs use in get_nth_most_common_value, also on int64_t. The long long std::abs (long long); overload is there only in C++11 and we in GCC10 still support C++98. Martin has said that a counter should never be INT64_MIN, so IMHO it is better to use abs_hwi which will assert that. 2020-03-11 Jakub Jelinek <jakub@redhat.com> PR bootstrap/93962 * value-prof.c (dump_histogram_value): Use abs_hwi instead of std::abs. (get_nth_most_common_value): Use abs_hwi instead of abs.
.