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

[Xin Tong via llvm-commits]

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