<div dir="ltr">Are you suggesting changing it to:<div><br></div><div><div style="font-size:12.8px">rotate_update_probability(floa<wbr>t orig_prob) {</div><div style="font-size:12.8px"><span style="font-size:12.8px"> if (orig_prob >= 0 && orig_prob < 0.5) {</span><br></div><div style="font-size:12.8px"> branch_prob_inside = 0;</div><div style="font-size:12.8px"><div style="font-size:12.8px"> branch_prob_outside = orig_prob;<br></div><div><span style="font-size:12.8px"> } else {</span><br></div></div><div style="font-size:12.8px"> branch_prob_inside = 2 - 1/orig_prob;<br></div><div style="font-size:12.8px"><div style="font-size:12.8px"> branch_prob_outside = 1;<br></div><div><span style="font-size:12.8px"> }</span><br></div></div><div style="font-size:12.8px">}</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">int estimate_trip_count(float latch_probability) {</div><div style="font-size:12.8px"> if (latch_probability == 0)</div><div style="font-size:12.8px"> return 0;</div><div style="font-size:12.8px"> else</div><div style="font-size:12.8px"> return 1/(1-latch_probability)</div><div style="font-size:12.8px">}</div></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">My gut feeling tells me this is a little dangerous. Micheal, what do you think?</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Dehao</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 12, 2017 at 2:33 PM, 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"><div class="HOEnZb"><div class="h5">On Thu, Jan 12, 2017 at 2:19 PM, Dehao Chen <<a href="mailto:danielcdh@gmail.com">danielcdh@gmail.com</a>> wrote:<br>
><br>
><br>
> On Thu, Jan 12, 2017 at 2:14 PM, Xin Tong <<a href="mailto:trent.xin.tong@gmail.com">trent.xin.tong@gmail.com</a>> wrote:<br>
>><br>
>> On Thu, Jan 12, 2017 at 2:10 PM, Dehao Chen <<a href="mailto:danielcdh@gmail.com">danielcdh@gmail.com</a>> wrote:<br>
>> ><br>
>> ><br>
>> > On Thu, Jan 12, 2017 at 2:02 PM, Xin Tong <<a href="mailto:trent.xin.tong@gmail.com">trent.xin.tong@gmail.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Thu, Jan 12, 2017 at 1:34 PM, Dehao Chen <<a href="mailto:danielcdh@gmail.com">danielcdh@gmail.com</a>><br>
>> >> wrote:<br>
>> >> > If we assume that the trip count will always be the same, then if<br>
>> >> > back<br>
>> >> > edge<br>
>> >> > taken probability falls in the range of (0, 0.5), then it already<br>
>> >> > broke<br>
>> >> > this<br>
>> >> > assumption. So we do not have enough info to infer both branch<br>
>> >> > probabilities. So probably we should simply set the inner probability<br>
>> >> > as<br>
>> >> > "0". And when we estimate loop trip count, we will always return 0 if<br>
>> >> > the<br>
>> >> > latch probability is 0. Something like:<br>
>> >> ><br>
>> >> > rotate_update_probability(<wbr>float orig_prob) {<br>
>> >> > branch_prob_outside = orig_prob;<br>
>> >> > if (orig_prob >= 0 && orig_prob < 0.5) {<br>
>> >> > branch_prob_inside = 0;<br>
>> >> > } else {<br>
>> >> > branch_prob_inside = 2 - 1/orig_prob;<br>
>> >> > }<br>
>> >> > }<br>
>> >> ><br>
>> >> > int estimate_trip_count(float latch_probability) {<br>
>> >> > if (latch_probability == 0)<br>
>> >> > return 0;<br>
>> >> > else<br>
>> >> > return 1/(1-latch_probability)<br>
>> >> > }<br>
>> >><br>
>> >> We assume the trip count will be close to each other, they do not have<br>
>> >> to be identical =). And that assumption has implications, I addressed<br>
>> >> the implications when we have polarized loop tripcounts in an earlier<br>
>> >> response. I feel we need to make assumptions and also reason about the<br>
>> >> fallouts if that assumption turns out to be not true as Michael said.<br>
>> ><br>
>> ><br>
>> > Yes, the above code is trying to address one case where the assumption<br>
>> > is<br>
>> > untrue.<br>
>> ><br>
>> >><br>
>> >><br>
>> >> Back to your example, I am not sure i understand it fully, is<br>
>> >> orig_prob the probability of branching outside before rotation ?<br>
>> ><br>
>> ><br>
>> > Yes, orig_prob is the original probability of the branch before loop<br>
>> > rotation.<br>
>><br>
>> If orig_prob == 0.5, the branch_prob_inside is 0. (branch_prob_inside<br>
>> is the probability that the loop body is branched to before rotation<br>
>> ?).<br>
><br>
><br>
> Sorry about the confusion. Let me use my original example:<br>
><br>
> Before loop rotation<br>
><br>
> L1:<br>
> if (cond) { // "orig_prob"<br>
> stmt;<br>
> goto L1;<br>
> }<br>
><br>
> After loop rotation:<br>
><br>
> if (cond) { // "branch_prob_outside"<br>
> L1:<br>
> stmt;<br>
> if (cond) { // "branch_prob_inside" // and also "latch_probability"<br>
> goto L1;<br>
> }<br>
> }<br>
><br>
> Note that all probability are for when "cond" is true.<br>
><br>
> Thanks,<br>
> Dehao<br>
<br>
</div></div>Thanks for the code. One thing I notice is that branch_prob_outside<br>
probably should not be orig_prob. orig_prob before rotation is the (#<br>
of times the exit is taken)/(# of times the header branch is hit<br>
through forward and back edges). So if the loop has a cmp i32 %index,<br>
127 and recurrence of +1 (index = index + 1), i.e. the loop has a trip<br>
count of 128, then branch_prob_outside = orig_prob = 1/128. However,<br>
branch_prob_outside should really be 0 as the loop will always<br>
execute, i..e branch_prob_outside in the guard block will never be<br>
taken.<br>
<div class="HOEnZb"><div class="h5">><br>
><br>
>><br>
>> Thanks,<br>
>> -Xin<br>
>> ><br>
>> >><br>
>> >><br>
>> >> Thanks,<br>
>> >> -Xin<br>
>> >><br>
>> >> ><br>
>> >> ><br>
>> >> > On Thu, Jan 12, 2017 at 12:59 PM, Michael Kuperstein via Phabricator<br>
>> >> > <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br>
>> >> >><br>
>> >> >> mkuper added inline comments.<br>
>> >> >><br>
>> >> >><br>
>> >> >> ================<br>
>> >> >> Comment at: lib/Transforms/Scalar/<wbr>LoopRotation.cpp:476<br>
>> >> >> + // data.<br>
>> >> >> + if (!<wbr>updateLoopEstimatedBranchWeigh<wbr>t(L, GBI, BI /* OrigHeader BR<br>
>> >> >> */)) {<br>
>> >> >> + BI->setMetadata(LLVMContext::<wbr>MD_prof, nullptr);<br>
>> >> >> ----------------<br>
>> >> >> Also, regardless of the rest of the discussion - I don't think we<br>
>> >> >> should<br>
>> >> >> drop the metadata on the floor if we fail.<br>
>> >> >><br>
>> >> >> I don't think "No data is better than imprecise data" is right in<br>
>> >> >> the<br>
>> >> >> general case, but that's arguable. Specifically here, though, we're<br>
>> >> >> imprecise even if updateLoopEstimatedBranchWeigh<wbr>t() succeeds,<br>
>> >> >> because<br>
>> >> >> of the<br>
>> >> >> assumptions we make on the distribution.<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>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br></div>