[ProfileData] Compute sum in annotateValueSite (NFC)

[kazutakahirata]

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.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

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 --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));