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

[Diego Novillo via llvm-commits]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151216/c5894202/attachment.html>