[PATCH] D15489: Use getEdgeProbability() instead of getEdgeWeight() in BFI and remove getEdgeWeight() interfaces from MBPI.

[Xinliang David Li via llvm-commits]

Because instr-based PGO does that too, I think.

David

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