------------------------------------------------------------------ Sender:Richard Biener <richard.guent...@gmail.com> Sent at:2018 Oct 31 (Wed) 17:11 To:bin.cheng <bin.ch...@linux.alibaba.com>; Jan Hubicka <hubi...@ucw.cz> Cc:GCC Patches <gcc-patches@gcc.gnu.org> Subject:Re: [PATCH AutoFDO/2]Treat ZERO as common profile probability/count > > > On Wed, Oct 31, 2018 at 7:30 AM bin.cheng <bin.ch...@linux.alibaba.com> wrote: >> >> Hi, >> In new profile probability/count infra, we have different precision quality >> categories, >> and probabilities/counts of different categories are not supposed to be >> compared or >> calculated. Though in general is an improvement, it introduces unexpected >> behavior. >> Specifically, class profile_probablity and profile_count themselves are >> implemented >> by comparing probabilities/counts against profile_count::zero(). while >> zero() is of >> profile_precision category, it's always compared different to zero of other >> precision >> categories including afdo. >> >> I can see two ways fixing this: 1) Treat zero as a common probability/count >> regardless >> of its category; 2) Provide an "is_zero" method rather than relying on "==" >> comparison >> against probability_count::zero(). 2) requires lots of code changes so I >> went with 1) >> in this patch set. This patch doesn't handle "always" but it might be. >> >> This patch also corrects a minor issue where we try to invert an >> uninitialized value. >> >> Bootstrap and test on x86_64 in patch set. Is it OK? >
Thanks for all the reviews.
> I'll defer on the emit_store_flag_force change, likewise for the zero
As Jeff pointed out too, this is discarded now.
> handling in
> compares - I don't think zeros of different qualities should compare equal.
> Would compares against ::always() not have the very same issue?
> Likewise ::even(),
> ::likely(), etc.? Those always get guessed quality.
In this version, I went with the second method which adds never_p/zero_p
member function for probability and count. Check on ZERO value won't fail
unexpected now. The patch also makes some other changes that avoids
short-circuit returning on ZERO probability/count and let.
>
> The invert change looks OK to me. The related change to the always() API
> would
> suggest to replace guessed_always() with always (guessed) and also do similar
> changes throughout the whole API...
The invert part is split as a standalone patch. I think adding a default
precision
parameter to member functions is a little bit better than having various version
functions here, like you mentioned for from_gcov_type.
Bootstrap and test on x86_64 in patch set. It fixes all (13) ICE autofdo tests.
Thanks,
bin
2018-11-02 Bin Cheng <bin.ch...@linux.alibaba.com>
* profile-count.h (profile_probability::always): Add parameter.
(profile_probability::invert): Don't invert uninitialized probability.
2018-11-02 Bin Cheng <bin.ch...@linux.alibaba.com>
* profile-count.h (profile_probability::never_p): New.
(profile_probability::operator+, +=, -, -=, *, *=, /, /=): Check ZERO
probability using never_p.
(profile_probability::apply_scale): Likewise.
(profile_probability::apply): Check using uninitialized_p.
(profile_count::zero_p): New.
(profile_count::compatible_p): Check ZERO count using zero_p.
(profile_count::operator+, +=, <, >, <=, >=, apply_scale): Likewise.
(profile_count::apply_probability, probability_in): Likewise.
(profile_count::operator-, -=): Likewise. Don't quick return if RHS
profile_count is ZERO.
(profile_count::max): Don't quick return in case of ZERO count.
* profile-count.c (profile_count::to_frequency): Check ZERO profile
probability or count using never_p or zero_p.
(profile_count::to_cgraph_frequency, to_sreal_scale): Likewise.
(profile_count::adjust_for_ipa_scaling): Likewise.
(profile_count::combine_with_ipa_count): Likewise.
(profile_probability::combine_with_count): Likewise.
* bb-reorder.c (sanitize_hot_paths): Likewise.
* cfg.c (update_bb_profile_for_threading): Likewise.
(scale_bbs_frequencies_profile_count): Likewise.
* ipa-profile.c (ipa_propagate_frequency_1): Likewise.
(ipa_propagate_frequency): Likewise.
* ipa-utility.c (ipa_merge_profiles): Likewise.
* predict.c (maybe_hot_count_p, probably_never_executed): Likewise.
(probably_never_executed_bb_p, unlikely_executed_stmt_p): Likewise.
(combine_predictions_for_bb): Likewise.
(drop_profile, handle_missing_profiles): Likewise.
(propagate_unlikely_bbs_forward): Likewise.
(determine_unlikely_bbs): Likewise.
(estimate_bb_frequencies): Likewise.
(compute_function_frequency, force_edge_cold): Likewise.
* profile.c (compute_branch_probabilities): Likewise.
* shrink-wrap.c (try_shrink_wrapping): Likewise.
* tree-ssa-loop-manip.c (scale_dominated_blocks_in_loop): Likewise.
(tree_transform_and_unroll_loop): Likewise.
* tree-ssa-threadupdate.c (update_profile): Likewise.
* tree-vect-loop.c (scale_profile_for_vect_loop): Likewise.
0002-Dont-invert-uninitialized-value.patch
Description: Binary data
0003-Check-zero-probability-count-using-member-function.patch
Description: Binary data
