<div dir="ltr">I remember removing it at some point in the past because it was deemed unnecessary.  What changed?  I don't have a good recollection of why we did it at first.<div><br></div><div><br></div><div>Diego.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 16, 2015 at 12:43 PM, Xinliang David Li via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Given that BFI analysis always add 1 to zero weight, I don't expect it<br>
will have any performance impact when autoFDO starts to do the<br>
'correction' -- so autoFDO should just do it.<br>
<br>
David<br>
<div class="HOEnZb"><div class="h5"><br>
On Tue, Dec 15, 2015 at 5:01 PM, Dehao Chen <<a href="mailto:danielcdh@gmail.com">danielcdh@gmail.com</a>> wrote:<br>
> AutoFDO does not convert 0-weight to 1. In theory, converting 0-weight to 1<br>
> should not affect performance. I'll do some experiments to evaluate.<br>
><br>
> Dehao<br>
><br>
> On Tue, Dec 15, 2015 at 4:51 PM, Cong Hou <<a href="mailto:congh@google.com">congh@google.com</a>> wrote:<br>
>><br>
>> On Tue, Dec 15, 2015 at 10:52 AM, Xinliang David Li via llvm-commits<br>
>> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>>><br>
>>> In most of the cases, BFI can rebuild BB weights from probability (plus<br>
>>> entry count) correctly, but there are exceptions including 1) functions with<br>
>>> irreducible loops; and 2) lossy profile data in AutoFDO case.<br>
>>><br>
>>> For the example CFG, without Cong's patch, BFI can recover the BB2's<br>
>>> weight -- but this is not by design but based on the assumption that the<br>
>>> weight of backedge is not scaled (and still keep the original value of the<br>
>>> sample count). However since BFI will force BB2->BB3 to have have weight 1,<br>
>>> the iteration count of the loop will be 1000 which might be lower than<br>
>>> actual.  With Cong's patch, the weight of BB2 recomputed can be way higher<br>
>>> than actual (as it is using probability 1's nominator value).<br>
>>><br>
>>> I think AutoFDO code needs to consider the limitation of BFI when<br>
>>> annotating loops -- basically if a loop has only single exit -- the exit<br>
>>> edge's weight can not be zero.<br>
>><br>
>><br>
>> If we don't have 0 edge weight in AutoFDO and other weights are not very<br>
>> high, then we should not get very large loop count even with this patch. I<br>
>> think we have already transform all zero-weights into ones in AutoFDO,<br>
>> right?<br>
>><br>
>> Cong<br>
>><br>
>>><br>
>>><br>
>>> David<br>
>>><br>
>>> On Tue, Dec 15, 2015 at 10:22 AM, Dehao Chen via llvm-commits<br>
>>> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>>>><br>
>>>> For the following CFG:<br>
>>>><br>
>>>> BB1(weight:100)<br>
>>>> BB2(weight:1000)<br>
>>>> BB3(weight:100)<br>
>>>><br>
>>>> BB1->BB2(probability: 0%)<br>
>>>> BB1->BB3(probability: 100%)<br>
>>>> BB2->BB2(probability: 100%)<br>
>>>> BB2->BB3(probability: 0%)<br>
>>>><br>
>>>> Is there a way we can rebuild BB weights from probability?<br>
>>>> Dehao<br>
>>>><br>
>>>> On Mon, Dec 14, 2015 at 9:58 PM, David Li <<a href="mailto:davidxl@google.com">davidxl@google.com</a>> wrote:<br>
>>>> > davidxl added a comment.<br>
>>>> ><br>
>>>> > Ok.  This is a case where loop exit edge is 'never' taken where the<br>
>>>> > loop backedge is executed. With instrumentation based PGO, even without FE<br>
>>>> > fix up of the 0 weight edge, such as scenario (aka, bad meta data) will<br>
>>>> > never occur, so whatever output of BFI will be fine.<br>
>>>> ><br>
>>>> > However, with AutoFDO, such MD_prof data can actually be generated .<br>
>>>> > For instance, the function has a single BB loop, and only the BB in the loop<br>
>>>> > has some samples but the entry/exit BB has none.<br>
>>>> ><br>
>>>> > The BFI result before this patch will most likely under-estimate the<br>
>>>> > loop trip count, while with this patch, it will most likely over-estimate<br>
>>>> > it. I think neither results are ideal, and AutoFDO needs to do something<br>
>>>> > (apply some heuristic) to prevent such case from being generated.  +dehao<br>
>>>> > and +dnovillo for comments.<br>
>>>> ><br>
>>>> ><br>
>>>> > <a href="http://reviews.llvm.org/D15489" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15489</a><br>
>>>> ><br>
>>>> ><br>
>>>> ><br>
>>>> _______________________________________________<br>
>>>> llvm-commits mailing list<br>
>>>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
>>><br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
>>><br>
>><br>
><br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>