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

Xin Tong via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 14:33:18 PST 2017


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


More information about the llvm-commits mailing list