[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/PerfReader.cpp:447
+
+  emitWarningSummary(
+      AllUntrackedCallsites.size(), SampleCounters.size(),
----------------
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. 


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:1043
+
+static const std::string EndNotBoundaryMsg =
+    "Range end is not on instruction boundary.";
----------------
hoy wrote:
> Make these static fields of PerfScriptReader? `static const char*` should be fine.
Sounds good, thanks!


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111902