<div dir="ltr">Dehao and I spent some time staring at this on a whiteboard, and we don't have a good conclusion.<div>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.</div><div><br></div><div>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.</div><div>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)...</div><div><br></div><div>We'll stare at this some more later. </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 12, 2017 at 10:37 AM, Xin Tong <span dir="ltr"><<a href="mailto:trent.xin.tong@gmail.com" target="_blank">trent.xin.tong@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Jan 12, 2017 at 10:29 AM, Xin Tong <<a href="mailto:trent.xin.tong@gmail.com">trent.xin.tong@gmail.com</a>> wrote:<br>
> On Thu, Jan 12, 2017 at 10:26 AM, Dehao Chen <<a href="mailto:danielcdh@gmail.com">danielcdh@gmail.com</a>> wrote:<br>
>><br>
>><br>
>> On Thu, Jan 12, 2017 at 9:26 AM, Michael Kuperstein <<a href="mailto:mkuper@google.com">mkuper@google.com</a>><br>
>> wrote:<br>
>>><br>
>>> I haven't looked at the patch yet, (and I'm also not at the office), but:<br>
>>><br>
>>> 1) The new probabilities should be the opposite of the last example Dehao<br>
>>> had. That is, the external if should be 1/7, and the one on the backedge<br>
>>> 7/28.<br>
>><br>
>><br>
>> For the 2nd case, the expected trip count is 0.2, thus it's not<br>
>> straight-forward to me how to set this. But if we make the expected trip<br>
>> count as 5, then looks to me the external if should be 5/1, and the one on<br>
>> the backedge should be 4/1?<br>
>><br>
>> And please make sure that when the input probability is the same, the output<br>
>> probability should also be the same. E.g., if you annotate the branch as (5,<br>
>> 1), and (10, 2), after the transformation, the probabilities should be<br>
>> equivalent.<br>
>><br>
>>><br>
>>> 2) I think this is just conditional probability - cond hasn't changed, but<br>
>>> P(cond | cond has been true once) is lower than P(cond). This exactly<br>
>>> corresponds to lowering the number of expected iterations by 1, given that<br>
>>> we know we already executed one iteration.<br>
>><br>
>><br>
>> Seem with this patch, when we compute the estimated trip count, we need to<br>
>> first check if it's rotated or not: if yes, we use (BE.taken/BE.not_taken) +<br>
>> 1, otherwise, we use (BE.taken/BE.not_taken). Is that true?<br>
><br>
<br>
</span>Sorry, Dehao I feel i talked past you, our current way of computing<br>
estimated trip count requires loop to be rotated.<br>
Yes, in the general case, the trip count of the loop depends on<br>
whether the loop is rotated or not as you said.<br>
<div class="HOEnZb"><div class="h5"><br>
> The way we compute EstimatedTripCount requires the loop to be rotated.<br>
> i.e. we only look at the latch block and expect it to be bottom<br>
> tested.<br>
>><br>
>> Dehao<br>
>><br>
>>><br>
>>><br>
>>><br>
>>> On Jan 12, 2017 09:21, "Xin Tong via Phabricator"<br>
>>> <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br>
>>><br>
>>> trentxintong added a comment.<br>
>>><br>
>>> @danielcdh Its the true the cond is not changed, but the inputs to the<br>
>>> condition has changed, before you were testing with index variable in the<br>
>>> loop header, now u have 2 conditions and the SSA values you use in the<br>
>>> conditions have changed, so the probability needs to be adjusted. I will<br>
>>> explain more when i get to office.<br>
>>><br>
>>><br>
>>> <a href="https://reviews.llvm.org/D28593" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D28593</a><br>
>>><br>
>>><br>
>>><br>
>>><br>
>><br>
</div></div></blockquote></div><br></div>