On Wed, Jan 20, 2021 at 5:25 PM Martin Liška <mli...@suse.cz> wrote: > > On 1/20/21 5:00 PM, Jan Hubicka wrote: > > There are two thinks that I would like to discuss first > > 1) I think the option is stil used for value profiling of divisors > > It's not. Right now the only usage lives in get_nth_most_common_value which > is an entry point being used for stringops, indirect call and divmod > histograms. > > > 2) it is not clear to me how the new counter establishes > > reproducibility for indiect calls that have more than 32 targets during > > the train run. > > We cannot ensure, but I would say that it's very unlikely to happen. > In case of Firefox, there are definitely other reasons why the build is not > reproducible. > I would expect arc counters to differ in between multiple training runs. > > If it's really a problem we can come up with other approaches: > - GCOV run-time control over # of tracked values (32 right now) > - similarly we can save more values in .gcda files > > I'm sending updated version of the patch.
So the discussion tells me that we want the option and of course want to have it work. In the patch I see the =multithreaded enum part was not documented (I don't see how it differs from =parallel-runs?), so I wonder if we really need three states. That said, the option handling is indeed broken at the moment. While the implementation is not perfect, does it have some pieces that help? That said, why not simply fix option handling by adding the missing = to the option? Using -fprofile-reproducibleserial etc. work but before adding backwards compatibility aliases I'd say we change w/o them for GCC 11 and only if there are complaints introduce them (and eventually backport the option handling fix to GCC 10). Richard. > Martin
