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.
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
