[PATCH] D109713: [AutoFDO][llvm-profgen] Report zero count for unexecuted part of function code

Lei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 16 13:11:20 PDT 2021


wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:216
     uint64_t Count = Item.second;
     if (Boundaries.find(Begin) == Boundaries.end())
       Boundaries[Begin] = BoundaryPoint();
----------------
hoy wrote:
> wlei wrote:
> > hoy wrote:
> > > This initialization is probably not needed since you are adding the default constructor.
> > Sounds good, refactor the code to use `emplace`
> I actually meant you can just do something like below. Sorry for the confusion.
> 
> 
> ```
>     Boundaries[Begin].addBeginCount(Count);
>     Boundaries[End].addEndCount(Count);
>     if (Count == 0) {
>       BeginPoint.IsZeroRangeBegin = true;
>       EndPoint.IsZeroRangeEnd = true;
>     }
> ```
I see, that's clearer. Thanks!


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:251
+      if (Count == 0 && ZeroRangeDepth == 0)
+        BeginAddress = UINT64_MAX;
     }
----------------
hoy wrote:
> wlei wrote:
> > hoy wrote:
> > > Add a comment for this? Suppose we never have a zero-count range, does this change the default behavior?
> > Comment added. It will change our previous logic a little bit, i, e. previous one can generate new zero-count range.
> > For example, supposing we have two non-overlapping ranges
> > ```
> > [<---- 10 ---->]  
> >                             [<----20 ----->]
> > A              B             C             D
> > ```
> > 
> > Previously, it will add a zero range [B+1, C-1]. (Anyway, this extra zero range will be filtered later as you can see the code below).
> > 
> > After this change, the begin point B+1 is marked invalid(UINT64_MAX), so no extra zero range is added.
> > 
> > 
> > 
> Thanks for the explanation. It makes sense to me now. Would be great to include your example in the comment.
example added!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109713/new/

https://reviews.llvm.org/D109713



More information about the llvm-commits mailing list