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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:447
+
+  emitWarningSummary(
+      AllUntrackedCallsites.size(), SampleCounters.size(),
----------------
wlei wrote:
> hoy wrote:
> > How about emit summary before the detailed messages?
> It seems we need to align with `warnInvalidRange` where we need to first get the invalid number then emit summary. 
I see. Makes sense to leave it as is.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:404
 
+      if (MCDesc.isTerminator())
+        TerminatorOffsets.insert(Offset);
----------------
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? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111902