On Fri, Mar 27, 2015 at 4:09 PM, Xinliang David Li <xinliangli at gmail.com> wrote:> How about only removing the scaling limit when PGO is on? I don't see the > need for this change without PGO.This is what my patch does, and it's getting into issues. With the scaling limit gone, the frequencies propagated overflow 64bits, so they need to be scaled down to a 64bit space. To be on the safe side, my patch is mapping them down to a 32bit space, but I am squishing them too much on the lower end. So regions of the CFG that before had distinct temperatures are now showing up with frequency == 1. I need a better smoother for the mapping from the Scale64 floats down to 64bit (or 32bit) integers. Diego.
On Fri, Mar 27, 2015 at 1:12 PM, Diego Novillo <dnovillo at google.com> wrote:> On Fri, Mar 27, 2015 at 4:09 PM, Xinliang David Li <xinliangli at gmail.com> > wrote: > > How about only removing the scaling limit when PGO is on? I don't see the > > need for this change without PGO. > > This is what my patch does, and it's getting into issues. With the > scaling limit gone, the frequencies propagated overflow 64bits, so > they need to be scaled down to a 64bit space. >What I mean is only do this when real profile data is used? I don't see the patch has that check ?> > To be on the safe side, my patch is mapping them down to a 32bit > space, but I am squishing them too much on the lower end. So regions > of the CFG that before had distinct temperatures are now showing up > with frequency == 1. > > I need a better smoother for the mapping from the Scale64 floats down > to 64bit (or 32bit) integers. >This seems to show another weakness of the block frequency propagation -- it can not handle infinite loops. We need to think about how to handle it .. David> > Diego. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150327/7f61bfaf/attachment.html>
On Fri, Mar 27, 2015 at 1:36 PM, Xinliang David Li <xinliangli at gmail.com> wrote:> To be on the safe side, my patch is mapping them down to a 32bit >> space, but I am squishing them too much on the lower end. So regions >> of the CFG that before had distinct temperatures are now showing up >> with frequency == 1. >> >> I need a better smoother for the mapping from the Scale64 floats down >> to 64bit (or 32bit) integers. >> > > > This seems to show another weakness of the block frequency propagation -- > it can not handle infinite loops. We need to think about how to handle it > .. >I think we should just have a synthetic "max" and not balloon the range due to it. Then we can pin the max at some fixed multiple of the actual max when scaling it down. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150327/a5d1adc6/attachment.html>
On Fri, Mar 27, 2015 at 4:36 PM, Xinliang David Li <xinliangli at gmail.com> wrote:> > > On Fri, Mar 27, 2015 at 1:12 PM, Diego Novillo <dnovillo at google.com> wrote: >> >> On Fri, Mar 27, 2015 at 4:09 PM, Xinliang David Li <xinliangli at gmail.com> >> wrote: >> > How about only removing the scaling limit when PGO is on? I don't see >> > the >> > need for this change without PGO. >> >> This is what my patch does, and it's getting into issues. With the >> scaling limit gone, the frequencies propagated overflow 64bits, so >> they need to be scaled down to a 64bit space. > > > What I mean is only do this when real profile data is used? I don't see the > patch has that check ?Well, no. It's not really possible at the moment to determine whether the branch weights you get are coming from real profiles or not. It has no way of knowing and it would actually produce the same results. This patch works inside the frequency propagator. We could have a flag that tells the analysis about it, but there is no way of doing it automatically.> >> >> >> To be on the safe side, my patch is mapping them down to a 32bit >> space, but I am squishing them too much on the lower end. So regions >> of the CFG that before had distinct temperatures are now showing up >> with frequency == 1. >> >> I need a better smoother for the mapping from the Scale64 floats down >> to 64bit (or 32bit) integers. > > > > This seems to show another weakness of the block frequency propagation -- > it can not handle infinite loops. We need to think about how to handle it > ..It can, actually. On an infinite loop, the mass out of the loop is 0, the mass in the back edge is infinite. Diego.