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

[Xin Tong via llvm-commits]

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.

Back to your example, I am not sure i understand it fully, is
orig_prob the probability of branching outside before 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
>>
>>
>>
>