[PATCH] D111902: [llvm-profgen] Warn on invalid range and show warning summary
Wenlei He via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 2 16:30:27 PDT 2021
wenlei added inline comments.
================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:1038
+ return;
+ WithColor::warning() << format("%.2f", static_cast<double>(Num) * 100 / Total)
+ << "%(" << Num << "/" << Total
----------------
Suggest we rephrase the message.
```
warning: 0.05%(1120/2428958) cases with issue: Profile context truncated due to missing probe for call instruction.
```
->
```
warning: 0.05% (1120/2428958) of profiled contexts are truncated due to missing probe for call instruction.
```
```
warning: 0.00%(2/178637) cases with issue: Range does not belong to any functions, likely from external function.
```
->
```
warning: 0.00% (2/178637) of profiled ranges do not belong to any functions.
```
And specifically for invalid ranges, I thought we filter out external entries already, no? Did you check that they are actually from external functions?
================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:1081-1082
+ "Range does not belong to any functions, likely from external function.";
+ const char *RangeCrossFuncMsg =
+ "Fall through range should not cross function boundaries.";
+
----------------
For EndNotBoundaryMsg and RangeCrossFuncMsg, we can hint profile/ binary mismatch: likely due to profile and binary mismatch.
================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:1097
+
+ auto *FRange = Binary->findFuncRangeForOffset(StartOffset);
+ if (!FRange) {
----------------
could also check `addressIsCode(StartOffset)` to make sure start is also on instruction boundary?
================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:1099
+ if (!FRange) {
+ DanglingRange++;
+ WarnInvalidRange(StartOffset, EndOffset, DanglingRangeMsg);
----------------
"dangling" can be a confusing term. If this is truly external, we can say ExternalRange, or whatever that's more explicit.
================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:327
+ bool offsetIsBoundry(uint64_t Offset) {
+ return BranchOffsets.count(Offset) || RetOffsets.count(Offset) ||
----------------
nit: offsetIsTransfer - this would be consistent with others. flow transfer instructions includes call,ret,branch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111902/new/
https://reviews.llvm.org/D111902
More information about the llvm-commits
mailing list