[llvm] [ProfileData] Compute sum in annotateValueSite (NFC) (PR #95199)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 11 22:23:05 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-pgo
Author: Kazu Hirata (kazutakahirata)
<details>
<summary>Changes</summary>
getValueForSite computes the total count -- the total number of times
a given value site is visited. The problem is that, excluding tests,
annotateValueSite is the only place that needs the total count.
This patch moves the total count computation to annotateValueSite.
---
Full diff: https://github.com/llvm/llvm-project/pull/95199.diff
3 Files Affected:
- (modified) llvm/include/llvm/ProfileData/InstrProf.h (+10-23)
- (modified) llvm/lib/ProfileData/InstrProf.cpp (+4-2)
- (modified) llvm/unittests/ProfileData/InstrProfTest.cpp (+3-9)
``````````diff
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index dae2caf0181e4..1cff403664fed 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -869,18 +869,14 @@ struct InstrProfRecord {
inline uint32_t getNumValueDataForSite(uint32_t ValueKind,
uint32_t Site) const;
- /// Return the array of profiled values at \p Site. If \p TotalC
- /// is not null, the total count of all target values at this site
- /// will be stored in \c *TotalC.
+ /// Return the array of profiled values at \p Site.
inline std::unique_ptr<InstrProfValueData[]>
- getValueForSite(uint32_t ValueKind, uint32_t Site,
- uint64_t *TotalC = nullptr) const;
+ getValueForSite(uint32_t ValueKind, uint32_t Site) const;
/// Get the target value/counts of kind \p ValueKind collected at site
- /// \p Site and store the result in array \p Dest. Return the total
- /// counts of all target values at this site.
- inline uint64_t getValueForSite(InstrProfValueData Dest[], uint32_t ValueKind,
- uint32_t Site) const;
+ /// \p Site and store the result in array \p Dest.
+ inline void getValueForSite(InstrProfValueData Dest[], uint32_t ValueKind,
+ uint32_t Site) const;
/// Reserve space for NumValueSites sites.
inline void reserveSites(uint32_t ValueKind, uint32_t NumValueSites);
@@ -1065,34 +1061,25 @@ uint32_t InstrProfRecord::getNumValueDataForSite(uint32_t ValueKind,
}
std::unique_ptr<InstrProfValueData[]>
-InstrProfRecord::getValueForSite(uint32_t ValueKind, uint32_t Site,
- uint64_t *TotalC) const {
- uint64_t Dummy = 0;
- uint64_t &TotalCount = (TotalC == nullptr ? Dummy : *TotalC);
+InstrProfRecord::getValueForSite(uint32_t ValueKind, uint32_t Site) const {
uint32_t N = getNumValueDataForSite(ValueKind, Site);
- if (N == 0) {
- TotalCount = 0;
+ if (N == 0)
return std::unique_ptr<InstrProfValueData[]>(nullptr);
- }
auto VD = std::make_unique<InstrProfValueData[]>(N);
- TotalCount = getValueForSite(VD.get(), ValueKind, Site);
+ getValueForSite(VD.get(), ValueKind, Site);
return VD;
}
-uint64_t InstrProfRecord::getValueForSite(InstrProfValueData Dest[],
- uint32_t ValueKind,
- uint32_t Site) const {
+void InstrProfRecord::getValueForSite(InstrProfValueData Dest[],
+ uint32_t ValueKind, uint32_t Site) const {
uint32_t I = 0;
- uint64_t TotalCount = 0;
for (auto V : getValueSitesForKind(ValueKind)[Site].ValueData) {
Dest[I].Value = V.Value;
Dest[I].Count = V.Count;
- TotalCount = SaturatingAdd(TotalCount, V.Count);
I++;
}
- return TotalCount;
}
void InstrProfRecord::reserveSites(uint32_t ValueKind, uint32_t NumValueSites) {
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index d707c07e6dc50..0d7b0274bdd9f 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1271,11 +1271,13 @@ void annotateValueSite(Module &M, Instruction &Inst,
if (!NV)
return;
- uint64_t Sum = 0;
std::unique_ptr<InstrProfValueData[]> VD =
- InstrProfR.getValueForSite(ValueKind, SiteIdx, &Sum);
+ InstrProfR.getValueForSite(ValueKind, SiteIdx);
ArrayRef<InstrProfValueData> VDs(VD.get(), NV);
+ uint64_t Sum = 0;
+ for (const InstrProfValueData &V : VDs)
+ Sum = SaturatingAdd(Sum, V.Count);
annotateValueSite(M, Inst, VDs, Sum, ValueKind, MaxMDCount);
}
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index 924d848176e77..8acb0fa0c717a 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -867,13 +867,11 @@ TEST_P(InstrProfReaderWriterTest, icall_and_vtable_data_read_write) {
// First indirect site.
{
- uint64_t TotalC;
- auto VD = R->getValueForSite(IPVK_IndirectCallTarget, 0, &TotalC);
+ auto VD = R->getValueForSite(IPVK_IndirectCallTarget, 0);
EXPECT_EQ(VD[0].Count, 3U * getProfWeight());
EXPECT_EQ(VD[1].Count, 2U * getProfWeight());
EXPECT_EQ(VD[2].Count, 1U * getProfWeight());
- EXPECT_EQ(TotalC, 6U * getProfWeight());
EXPECT_STREQ((const char *)VD[0].Value, "callee3");
EXPECT_STREQ((const char *)VD[1].Value, "callee2");
@@ -882,13 +880,11 @@ TEST_P(InstrProfReaderWriterTest, icall_and_vtable_data_read_write) {
// First vtable site.
{
- uint64_t TotalC;
- auto VD = R->getValueForSite(IPVK_VTableTarget, 0, &TotalC);
+ auto VD = R->getValueForSite(IPVK_VTableTarget, 0);
EXPECT_EQ(VD[0].Count, 3U * getProfWeight());
EXPECT_EQ(VD[1].Count, 2U * getProfWeight());
EXPECT_EQ(VD[2].Count, 1U * getProfWeight());
- EXPECT_EQ(TotalC, 6U * getProfWeight());
EXPECT_EQ(VD[0].Value, getCalleeAddress(vtable3));
EXPECT_EQ(VD[1].Value, getCalleeAddress(vtable2));
@@ -897,12 +893,10 @@ TEST_P(InstrProfReaderWriterTest, icall_and_vtable_data_read_write) {
// Second vtable site.
{
- uint64_t TotalC;
- auto VD = R->getValueForSite(IPVK_VTableTarget, 1, &TotalC);
+ auto VD = R->getValueForSite(IPVK_VTableTarget, 1);
EXPECT_EQ(VD[0].Count, 2U * getProfWeight());
EXPECT_EQ(VD[1].Count, 1U * getProfWeight());
- EXPECT_EQ(TotalC, 3U * getProfWeight());
EXPECT_EQ(VD[0].Value, getCalleeAddress(vtable2));
EXPECT_EQ(VD[1].Value, getCalleeAddress(vtable1));
``````````
</details>
https://github.com/llvm/llvm-project/pull/95199
More information about the llvm-commits
mailing list