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

[Dehao Chen via llvm-commits]

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

> On Thu, Jan 12, 2017 at 2:10 PM, Dehao Chen <danielcdh at gmail.com> wrote:
> >
> >
> > 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.
>
> If orig_prob == 0.5, the branch_prob_inside is 0. (branch_prob_inside
> is the probability that the loop body is branched to before rotation
> ?).
>

Sorry about the confusion. Let me use my original example:

Before loop rotation

L1:
if (cond) { // "orig_prob"
  stmt;
  goto L1;
}

After loop rotation:

if (cond) { // "branch_prob_outside"
L1:
  stmt;
  if (cond) { // "branch_prob_inside" // and also "latch_probability"
    goto L1;
  }
}

Note that all probability are for when "cond" is true.

Thanks,
Dehao



> Thanks,
> -Xin
> >
> >>
> >>
> >> 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/a6830891/attachment.html>