[llvm] 00fa3fb - [ProfileData] Compute sum in annotateValueSite (NFC) (#95199)

[via llvm-commits]

Author: Kazu Hirata
Date: 2024-06-12T10:14:33-07:00
New Revision: 00fa3fbfb846756a902527cdd793f1ad7ad9369b

URL: https://github.com/llvm/llvm-project/commit/00fa3fbfb846756a902527cdd793f1ad7ad9369b
DIFF: https://github.com/llvm/llvm-project/commit/00fa3fbfb846756a902527cdd793f1ad7ad9369b.diff

LOG: [ProfileData] Compute sum in annotateValueSite (NFC) (#95199)

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.

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/InstrProf.h
    llvm/lib/ProfileData/InstrProf.cpp
    llvm/unittests/ProfileData/InstrProfTest.cpp

Removed: 
    


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