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

[Xin Tong via llvm-commits]

On Thu, Jan 12, 2017 at 10:26 AM, Dehao Chen <danielcdh at gmail.com> wrote:
>
>
> On Thu, Jan 12, 2017 at 9:26 AM, Michael Kuperstein <mkuper at google.com>
> wrote:
>>
>> I haven't looked at the patch yet, (and I'm also not at the office), but:
>>
>> 1) The new probabilities should be the opposite of the last example Dehao
>> had. That is, the external if should be 1/7, and the one on the backedge
>> 7/28.
>
>
> For the 2nd case, the expected trip count is 0.2, thus it's not
> straight-forward to me how to set this. But if we make the expected trip
> count as 5, then looks to me the external if should be 5/1, and the one on
> the backedge should be 4/1?
>
> And please make sure that when the input probability is the same, the output
> probability should also be the same. E.g., if you annotate the branch as (5,
> 1), and (10, 2), after the transformation, the probabilities should be
> equivalent.
>
>>
>> 2) I think this is just conditional probability - cond hasn't changed, but
>> P(cond | cond has been true once) is lower than P(cond). This exactly
>> corresponds to lowering the number of expected iterations by 1, given that
>> we know we already executed one iteration.
>
>
> Seem with this patch, when we compute the estimated trip count, we need to
> first check if it's rotated or not: if yes, we use (BE.taken/BE.not_taken) +
> 1, otherwise, we use (BE.taken/BE.not_taken). Is that true?

The way we compute EstimatedTripCount requires the loop to be rotated.
i.e. we only look at the latch block and expect it to be bottom
tested.
>
> Dehao
>
>>
>>
>>
>> On Jan 12, 2017 09:21, "Xin Tong via Phabricator"
>> <reviews at reviews.llvm.org> wrote:
>>
>> trentxintong added a comment.
>>
>> @danielcdh Its the true the cond is not changed, but the inputs to the
>> condition has changed, before you were testing with index variable in the
>> loop header, now u have 2 conditions and the SSA values you use in the
>> conditions have changed, so the probability needs to be adjusted. I will
>> explain more when i get to office.
>>
>>
>> https://reviews.llvm.org/D28593
>>
>>
>>
>>
>