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

[Hongtao Yu via Phabricator via llvm-commits]

hoy 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();
----------------
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;
    }
```


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:251
+      if (Count == 0 && ZeroRangeDepth == 0)
+        BeginAddress = UINT64_MAX;
     }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109713