[PATCH] D111902: [llvm-profgen] Warn on invalid range and show warning summary

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:404
 
+      if (MCDesc.isTerminator())
+        TerminatorOffsets.insert(Offset);
----------------
hoy wrote:
> wlei wrote:
> > hoy wrote:
> > > Is this supposed to identify a branch instruction? `BranchAddrs` may sound easier to understand?
> > isTerminator can be branches and return, refer to the comments from `isTerminator`
> > ```
> >   /// Returns true if this instruction part of the terminator for
> >   /// a basic block.  Typically this is things like return and branch
> >   /// instructions.
> >   ///
> > ```
> > 
> > Added comments on it.
> I see. Return instructions can also be considered branch instructions in the profile generator. They are basically jumps. We use `TerminatorOffsets` to check if the last instruction of a range is a jump, right? Terminator sounds a bit formal. Branches or jumps may be more natural to understand. What do you think? 
Yeah, this term is a little bit ambiguous.  I have the function to check if this is a jump, see below.
```
 bool offsetIsBoundry(uint64_t Offset) {
    return TerminatorOffsets.count(Offset) || CallAddrs.count(Offset);
  }
```
So `Terminator` is `branch` or `return` but not `call`. So we should check both `Terminato`r and `call` here. jump seems including `call`.

Maybe rename it to `BranchOrReturnOffsets`?





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111902