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

Dehao Chen via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 14:40:30 PST 2017


Are you suggesting changing it to:

rotate_update_probability(float orig_prob) {
  if (orig_prob >= 0 && orig_prob < 0.5) {
    branch_prob_inside = 0;
    branch_prob_outside = orig_prob;
  } else {
    branch_prob_inside = 2 - 1/orig_prob;
    branch_prob_outside = 1;
  }
}

int estimate_trip_count(float latch_probability) {
   if (latch_probability == 0)
     return 0;
   else
     return 1/(1-latch_probability)
}

My gut feeling tells me this is a little dangerous. Micheal, what do you
think?

Dehao

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

> On Thu, Jan 12, 2017 at 2:19 PM, Dehao Chen <danielcdh at gmail.com> wrote:
> >
> >
> > 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 for the code. One thing I notice is that branch_prob_outside
> probably should not be orig_prob. orig_prob before rotation is the (#
> of times the exit is taken)/(# of times the header branch is hit
> through forward and back edges). So if the loop has a cmp i32 %index,
> 127 and recurrence of +1 (index = index + 1), i.e. the loop has a trip
> count of 128, then branch_prob_outside = orig_prob = 1/128.  However,
> branch_prob_outside should really be 0 as the loop will always
> execute, i..e branch_prob_outside in the guard block will never be
> taken.
> >
> >
> >>
> >> 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/e65feb6d/attachment.html>


More information about the llvm-commits mailing list