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

[Michael Kuperstein via llvm-commits]

Dehao and I spent some time staring at this on a whiteboard, and we don't
have a good conclusion.
What loop rotation does now looks technically wrong, but it's not clear
what the right fix is. This is basically the same problem as we have inside
loop peeling - there isn't really enough information to assign
probabilities after rotation without knowing the actual distribution.

We can't just say "If the average trip of the original loop count is X, the
average trip count of the rotated loop is X-1" and update the edges
accordingly.
As Dehao's example showed, this breaks down, for example, when the average
trip count is below 1. It's even worse for the first iteration edge - the
probability is different for "a loop that is always taken 100 times" (in
which case it's 100/0), and "a loop that is taken 100 times on average -
half the time it's 0, half the time it's 200" (in which case it's 50/50)...

We'll stare at this some more later.

On Thu, Jan 12, 2017 at 10:37 AM, Xin Tong <trent.xin.tong at gmail.com> wrote:

> On Thu, Jan 12, 2017 at 10:29 AM, Xin Tong <trent.xin.tong at gmail.com>
> wrote:
> > 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?
> >
>
> Sorry, Dehao I feel i talked past you, our current way of computing
> estimated trip count requires loop to be rotated.
> Yes, in the general case, the trip count of the loop depends on
> whether the loop is rotated or not as you said.
>
> > 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
> >>>
> >>>
> >>>
> >>>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170112/66340d66/attachment.html>