-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[llvm-profdata] Clean up traverseAllValueSites (NFC) #95467
[llvm-profdata] Clean up traverseAllValueSites (NFC) #95467
Conversation
If NV == 0, nothing interesting happens after the "if" statement. We should just "continue" to the next value site. While I am at it, this patch migrates a use of getValueForSite to getValueArrayForSite.
@llvm/pr-subscribers-pgo Author: Kazu Hirata (kazutakahirata) ChangesIf NV == 0, nothing interesting happens after the "if" statement. We While I am at it, this patch migrates a use of getValueForSite to Full diff: https://github.com/llvm/llvm-project/pull/95467.diff 1 Files Affected:
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index fae6d1e989ab5..4c03e89d9e432 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -2695,30 +2695,30 @@ static void traverseAllValueSites(const InstrProfRecord &Func, uint32_t VK,
uint32_t NS = Func.getNumValueSites(VK);
Stats.TotalNumValueSites += NS;
for (size_t I = 0; I < NS; ++I) {
- uint32_t NV = Func.getNumValueDataForSite(VK, I);
- std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, I);
+ auto VD = Func.getValueArrayForSite(VK, I);
+ if (VD.empty())
+ continue;
+ uint32_t NV = VD.size();
Stats.TotalNumValues += NV;
- if (NV) {
- Stats.TotalNumValueSitesWithValueProfile++;
- if (NV > Stats.ValueSitesHistogram.size())
- Stats.ValueSitesHistogram.resize(NV, 0);
- Stats.ValueSitesHistogram[NV - 1]++;
- }
+ Stats.TotalNumValueSitesWithValueProfile++;
+ if (NV > Stats.ValueSitesHistogram.size())
+ Stats.ValueSitesHistogram.resize(NV, 0);
+ Stats.ValueSitesHistogram[NV - 1]++;
uint64_t SiteSum = 0;
- for (uint32_t V = 0; V < NV; V++)
- SiteSum += VD[V].Count;
+ for (const auto &V : VD)
+ SiteSum += V.Count;
if (SiteSum == 0)
SiteSum = 1;
- for (uint32_t V = 0; V < NV; V++) {
+ for (const auto &V : VD) {
OS << "\t[ " << format("%2u", I) << ", ";
if (Symtab == nullptr)
- OS << format("%4" PRIu64, VD[V].Value);
+ OS << format("%4" PRIu64, V.Value);
else
- OS << Symtab->getFuncOrVarName(VD[V].Value);
- OS << ", " << format("%10" PRId64, VD[V].Count) << " ] ("
- << format("%.2f%%", (VD[V].Count * 100.0 / SiteSum)) << ")\n";
+ OS << Symtab->getFuncOrVarName(V.Value);
+ OS << ", " << format("%10" PRId64, V.Count) << " ] ("
+ << format("%.2f%%", (V.Count * 100.0 / SiteSum)) << ")\n";
}
}
}
|
uint32_t NV = Func.getNumValueDataForSite(VK, I); | ||
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, I); | ||
auto VD = Func.getValueArrayForSite(VK, I); | ||
if (VD.empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: instead of calling both empty and size, just call 'size()'. if NV is 0, return.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the latest iteration.
If NV == 0, nothing interesting happens after the "if" statement. We should just "continue" to the next value site. While I am at it, this patch migrates a use of getValueForSite to getValueArrayForSite.
If NV == 0, nothing interesting happens after the "if" statement. We
should just "continue" to the next value site.
While I am at it, this patch migrates a use of getValueForSite to
getValueArrayForSite.