On Thu, 9 Jun 2016, Jan Hubicka wrote:
> Hi,
> after we read the profile, we know expected number of iterations.
We know the average ;) It may make sense to add some histogram
value profiling for niter now that we should easily able to do so.
> Currently we use profile each time estimate_numbers_of_iterations_loop
> is called to recompute this value. This is not very safe because the
> profile may be misupdated. It seems safer to compute it at once and
> maintain thorough the compilation.
>
> Notice that I removed:
> - /* Force estimate compuation but leave any existing upper bound in place.
> */
> - loop->any_estimate = false;
> From beggining of estimate_numbers_of_iterations_loop. I can not make sense
> of
> this. Even w/o profile if we have estimate, we are better to maintain it
> because
> later we may not be able to derive it again.
> There seems to be no code that is forced by setting loop->any_estimate = true.
> Only code that cares seems to be record_niter_bound that only decreases
> existing
> estimates. THis seems sane procedure - we don't roll loops.
>
> Bootstrapped/regtested x86_64-linux, OK?
Ok. Did you check what this does to SPEC with FDO?
Thanks,
Richard.
> Honza
>
> * profile.c: Include cfgloop.h.
> (branch_prob): Compute estimated number of iterations.
> * tree-ssa-loop-niter.c (estimate_numbers_of_iterations_loop): Do not
> recompute estimate number of iterations from profile.
> Index: profile.c
> ===================================================================
> --- profile.c (revision 237221)
> +++ profile.c (working copy)
> @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3.
> #include "gimple-iterator.h"
> #include "tree-cfg.h"
> #include "dumpfile.h"
> +#include "cfgloop.h"
>
> #include "profile.h"
>
> @@ -1329,9 +1330,21 @@ branch_prob (void)
> coverage_end_function (lineno_checksum, cfg_checksum);
> if (flag_branch_probabilities && profile_info)
> {
> + struct loop *loop;
> if (dump_file && (dump_flags & TDF_DETAILS))
> report_predictor_hitrates ();
> profile_status_for_fn (cfun) = PROFILE_READ;
> +
> + /* At this moment we have precise loop iteration count estimates.
> + Record them to loop structure before the profile gets out of date. */
> + FOR_EACH_LOOP (loop, 0)
> + if (loop->header->count)
> + {
> + gcov_type nit = expected_loop_iterations_unbounded (loop);
> + widest_int bound = gcov_type_to_wide_int (nit);
> + loop->any_estimate = false;
> + record_niter_bound (loop, bound, true, false);
> + }
> compute_function_frequency ();
> }
> }
> Index: tree-ssa-loop-niter.c
> ===================================================================
> --- tree-ssa-loop-niter.c (revision 237221)
> +++ tree-ssa-loop-niter.c (working copy)
> @@ -3721,8 +3721,26 @@ estimate_numbers_of_iterations_loop (str
> return;
>
> loop->estimate_state = EST_AVAILABLE;
> - /* Force estimate compuation but leave any existing upper bound in place.
> */
> - loop->any_estimate = false;
> +
> + /* If we have a measured profile, use it to estimate the number of
> + iterations. Normally this is recorded by branch_prob right after
> + reading the profile. In case we however found a new loop, record the
> + information here.
> +
> + Explicitly check for profile status so we do not report
> + wrong prediction hitrates for guessed loop iterations heuristics.
> + Do not recompute already recorded bounds - we ought to be better on
> + updating iteration bounds than updating profile in general and thus
> + recomputing iteration bounds later in the compilation process will just
> + introduce random roundoff errors. */
> + if (!loop->any_estimate
> + && loop->header->count != 0
> + && profile_status_for_fn (cfun) >= PROFILE_READ)
> + {
> + gcov_type nit = expected_loop_iterations_unbounded (loop);
> + bound = gcov_type_to_wide_int (nit);
> + record_niter_bound (loop, bound, true, false);
> + }
>
> /* Ensure that loop->nb_iterations is computed if possible. If it turns
> out
> to be constant, we avoid undefined behavior implied bounds and instead
> @@ -3756,17 +3774,6 @@ estimate_numbers_of_iterations_loop (str
>
> maybe_lower_iteration_bound (loop);
>
> - /* If we have a measured profile, use it to estimate the number of
> - iterations. Explicitly check for profile status so we do not report
> - wrong prediction hitrates for guessed loop iterations heuristics. */
> - if (loop->header->count != 0
> - && profile_status_for_fn (cfun) >= PROFILE_READ)
> - {
> - gcov_type nit = expected_loop_iterations_unbounded (loop);
> - bound = gcov_type_to_wide_int (nit);
> - record_niter_bound (loop, bound, true, false);
> - }
> -
> /* If we know the exact number of iterations of this loop, try to
> not break code with undefined behavior by not recording smaller
> maximum number of iterations. */
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
