On Fri, Mar 9, 2018 at 10:51 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Fri, Mar 9, 2018 at 10:25 AM, Paul Hua <paul.hua...@gmail.com> wrote: >> It's looks fixed >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82965#c12 on mips64el. > Hmm, is it fixed? or is it exposed now on mips64el? I read the latter > from the comment.
It is fixed, Bootstrap and test passed on mips64el. Thanks, Paul Hua > I think the issue is like explained, but haven't dug into when/why it > is triggered in vect_peeling only for some targets. > Honza asked couple questions about my patch offline, I will revisit > the patch, see how to address > his concern. > > Thanks, > bin >> >> Thanks. >> >> On Mon, Feb 26, 2018 at 8:02 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: >>> Ping^2 >>> >>> Thanks, >>> bin >>> >>> On Mon, Feb 19, 2018 at 5:14 PM, Jakub Jelinek <ja...@redhat.com> wrote: >>>> Hi! >>>> >>>> Honza, do you think you could have a look at this P1 fix? >>>> >>>> Thanks. >>>> >>>> On Wed, Jan 31, 2018 at 10:03:51AM +0000, Bin Cheng wrote: >>>>> Hi, >>>>> This patch fixes invalid profile count information in vectorization >>>>> peeling. >>>>> Current implementation is a bit confusing for me since it tries to compute >>>>> an overall probability based on scaling probability and change of >>>>> estimated >>>>> niters. This patch does it in two steps. Firstly it does the scaling, >>>>> then >>>>> it adjusts to new estimated niters by simply adjusting loop's latch count >>>>> information; scaling the loop's count information by the proportion >>>>> new_estimated_niters/old_estimate_niters. Of course we have to adjust >>>>> loop >>>>> latch's count information back after scaling. >>>>> >>>>> Bootstrap and test on x86_64 and AArch64. gcc.dg/vect/pr79347.c is fixed >>>>> for both PR82965 and PR83991. Is this OK? >>>>> >>>>> Thanks, >>>>> bin >>>>> >>>>> 2018-01-30 Bin Cheng <bin.ch...@arm.com> >>>>> >>>>> PR tree-optimization/82965 >>>>> PR tree-optimization/83991 >>>>> * cfgloopmanip.c (scale_loop_profile): Further scale loop's profile >>>>> information if the loop was predicted to iterate too many times. >>>> >>>>> diff --git a/gcc/cfgloopmanip.c b/gcc/cfgloopmanip.c >>>>> index b9b76d8..1f560b8 100644 >>>>> --- a/gcc/cfgloopmanip.c >>>>> +++ b/gcc/cfgloopmanip.c >>>>> @@ -509,7 +509,7 @@ scale_loop_profile (struct loop *loop, >>>>> profile_probability p, >>>>> gcov_type iteration_bound) >>>>> { >>>>> gcov_type iterations = expected_loop_iterations_unbounded (loop); >>>>> - edge e; >>>>> + edge e, preheader_e; >>>>> edge_iterator ei; >>>>> >>>>> if (dump_file && (dump_flags & TDF_DETAILS)) >>>>> @@ -521,77 +521,66 @@ scale_loop_profile (struct loop *loop, >>>>> profile_probability p, >>>>> (int)iteration_bound, (int)iterations); >>>>> } >>>>> >>>>> + /* Scale the probabilities. */ >>>>> + scale_loop_frequencies (loop, p); >>>>> + >>>>> /* See if loop is predicted to iterate too many times. */ >>>>> - if (iteration_bound && iterations > 0 >>>>> - && p.apply (iterations) > iteration_bound) >>>>> + if (iteration_bound == 0 || iterations <= 0 >>>>> + || p.apply (iterations) <= iteration_bound) >>>>> + return; >>>>> + >>>>> + e = single_exit (loop); >>>>> + preheader_e = loop_preheader_edge (loop); >>>>> + profile_count count_in = preheader_e->count (); >>>>> + if (e && preheader_e >>>>> + && count_in > profile_count::zero () >>>>> + && loop->header->count.initialized_p ()) >>>>> { >>>>> - /* Fixing loop profile for different trip count is not trivial; >>>>> the exit >>>>> - probabilities has to be updated to match and frequencies >>>>> propagated down >>>>> - to the loop body. >>>>> - >>>>> - We fully update only the simple case of loop with single exit that >>>>> is >>>>> - either from the latch or BB just before latch and leads from BB >>>>> with >>>>> - simple conditional jump. This is OK for use in vectorizer. */ >>>>> - e = single_exit (loop); >>>>> - if (e) >>>>> - { >>>>> - edge other_e; >>>>> - profile_count count_delta; >>>>> + edge other_e; >>>>> + profile_count count_delta; >>>>> >>>>> - FOR_EACH_EDGE (other_e, ei, e->src->succs) >>>>> - if (!(other_e->flags & (EDGE_ABNORMAL | EDGE_FAKE)) >>>>> - && e != other_e) >>>>> - break; >>>>> + FOR_EACH_EDGE (other_e, ei, e->src->succs) >>>>> + if (!(other_e->flags & (EDGE_ABNORMAL | EDGE_FAKE)) >>>>> + && e != other_e) >>>>> + break; >>>>> >>>>> - /* Probability of exit must be 1/iterations. */ >>>>> - count_delta = e->count (); >>>>> - e->probability = profile_probability::always () >>>>> + /* Probability of exit must be 1/iterations. */ >>>>> + count_delta = e->count (); >>>>> + e->probability = profile_probability::always () >>>>> .apply_scale (1, iteration_bound); >>>>> - other_e->probability = e->probability.invert (); >>>>> - count_delta -= e->count (); >>>>> - >>>>> - /* If latch exists, change its count, since we changed >>>>> - probability of exit. Theoretically we should update >>>>> everything from >>>>> - source of exit edge to latch, but for vectorizer this is >>>>> enough. */ >>>>> - if (loop->latch >>>>> - && loop->latch != e->src) >>>>> - { >>>>> - loop->latch->count += count_delta; >>>>> - } >>>>> - } >>>>> + other_e->probability = e->probability.invert (); >>>>> >>>>> /* Roughly speaking we want to reduce the loop body profile by the >>>>> difference of loop iterations. We however can do better if >>>>> we look at the actual profile, if it is available. */ >>>>> - p = p.apply_scale (iteration_bound, iterations); >>>>> - >>>>> - if (loop->header->count.initialized_p ()) >>>>> - { >>>>> - profile_count count_in = profile_count::zero (); >>>>> + p = profile_probability::always (); >>>>> >>>>> - FOR_EACH_EDGE (e, ei, loop->header->preds) >>>>> - if (e->src != loop->latch) >>>>> - count_in += e->count (); >>>>> - >>>>> - if (count_in > profile_count::zero () ) >>>>> - { >>>>> - p = count_in.probability_in (loop->header->count.apply_scale >>>>> - (iteration_bound, 1)); >>>>> - } >>>>> - } >>>>> + count_in = count_in.apply_scale (iteration_bound, 1); >>>>> + p = count_in.probability_in (loop->header->count); >>>>> if (!(p > profile_probability::never ())) >>>>> p = profile_probability::very_unlikely (); >>>>> - } >>>>> >>>>> - if (p >= profile_probability::always () >>>>> - || !p.initialized_p ()) >>>>> - return; >>>>> + if (p >= profile_probability::always () >>>>> + || !p.initialized_p ()) >>>>> + return; >>>>> >>>>> - /* Scale the actual probabilities. */ >>>>> - scale_loop_frequencies (loop, p); >>>>> - if (dump_file && (dump_flags & TDF_DETAILS)) >>>>> - fprintf (dump_file, ";; guessed iterations are now %i\n", >>>>> - (int)expected_loop_iterations_unbounded (loop)); >>>>> + /* If latch exists, change its count, since we changed >>>>> + probability of exit. Theoretically we should update everything >>>>> from >>>>> + source of exit edge to latch, but for vectorizer this is enough. >>>>> */ >>>>> + if (loop->latch && loop->latch != e->src) >>>>> + loop->latch->count += count_delta; >>>>> + >>>>> + /* Scale the probabilities. */ >>>>> + scale_loop_frequencies (loop, p); >>>>> + >>>>> + /* Change latch's count back. */ >>>>> + if (loop->latch && loop->latch != e->src) >>>>> + loop->latch->count -= count_delta; >>>>> + >>>>> + if (dump_file && (dump_flags & TDF_DETAILS)) >>>>> + fprintf (dump_file, ";; guessed iterations are now %i\n", >>>>> + (int)expected_loop_iterations_unbounded (loop)); >>>>> + } >>>>> } >>>>> >>>>> /* Recompute dominance information for basic blocks outside LOOP. */ >>>> >>>> >>>> Jakub