[PATCH] D28593: Update loop branch_weight metadata after loop rotation.

[Dehao Chen via llvm-commits]

On Thu, Jan 12, 2017 at 2:02 PM, Xin Tong <trent.xin.tong at gmail.com> wrote:

> On Thu, Jan 12, 2017 at 1:34 PM, Dehao Chen <danielcdh at gmail.com> wrote:
> > If we assume that the trip count will always be the same, then if back
> edge
> > taken probability falls in the range of (0, 0.5), then it already broke
> this
> > assumption. So we do not have enough info to infer both branch
> > probabilities. So probably we should simply set the inner probability as
> > "0". And when we estimate loop trip count, we will always return 0 if the
> > latch probability is 0. Something like:
> >
> > rotate_update_probability(float orig_prob) {
> >   branch_prob_outside = orig_prob;
> >   if (orig_prob >= 0 && orig_prob < 0.5) {
> >     branch_prob_inside = 0;
> >   } else {
> >     branch_prob_inside = 2 - 1/orig_prob;
> >   }
> > }
> >
> > int estimate_trip_count(float latch_probability) {
> >    if (latch_probability == 0)
> >      return 0;
> >    else
> >      return 1/(1-latch_probability)
> > }
>
> We assume the trip count will be close to each other, they do not have
> to be identical =). And that assumption has implications, I addressed
> the implications when we have polarized loop tripcounts in an earlier
> response. I feel we need to make assumptions and also reason about the
> fallouts if that assumption turns out to be not true as Michael said.
>

Yes, the above code is trying to address one case where the assumption is
untrue.


>
> Back to your example, I am not sure i understand it fully, is
> orig_prob the probability of branching outside before rotation ?
>

Yes, orig_prob is the original probability of the branch before loop
rotation.


>
> Thanks,
> -Xin
>
> >
> >
> > On Thu, Jan 12, 2017 at 12:59 PM, Michael Kuperstein via Phabricator
> > <reviews at reviews.llvm.org> wrote:
> >>
> >> mkuper added inline comments.
> >>
> >>
> >> ================
> >> Comment at: lib/Transforms/Scalar/LoopRotation.cpp:476
> >> +  // data.
> >> +  if (!updateLoopEstimatedBranchWeight(L, GBI, BI /* OrigHeader BR
> */)) {
> >> +    BI->setMetadata(LLVMContext::MD_prof, nullptr);
> >> ----------------
> >> Also, regardless of the rest of the discussion - I don't think we should
> >> drop the metadata on the floor if we fail.
> >>
> >> I don't think "No data is better than imprecise data" is right in the
> >> general case, but that's arguable. Specifically here, though, we're
> >> imprecise even if updateLoopEstimatedBranchWeight() succeeds, because
> of the
> >> assumptions we make on the distribution.
> >>
> >>
> >> https://reviews.llvm.org/D28593
> >>
> >>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170112/7ab3ba85/attachment-0001.html>