[llvm] [ProfileData] Use std::vector for ValueData (NFC) (PR #95194)

Kazu Hirata via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 11 21:25:17 PDT 2024


https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/95194

This patch changes the type of ValueData to
std::vector<InstrProfValueData> so that, in a follow-up patch, we can
teach getValueForSite to return ArrayRef<InstrProfValueData>.

Currently, a typical traversal over the value data looks like:

  uint32_t NV = Func.getNumValueDataForSite(VK, I);
  std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, I);
  for (uint32_t V = 0; V < NV; V++)
    Do something with VD[V].Value and/or VD[V].Count;

Note that we need to call getNumValueDataForSite and getValueForSite
separately.  If getValueForSite returns ArrayRef<InstrProfValueData>
in the future, then we'll be able to do something like:

  for (const auto &V : Func.getValueForSite(VK, I))
    Do something with V.Value and/or V.Count;

If ArrayRef<InstrProfValueData> directly points to ValueData, then
getValueForSite won't need to allocate memory with std::make_unique.

Now, switching to std::vector requires us to update several places:

- sortByTargetValues switches to llvm::sort because we don't need to
  worry about sort stability.

- sortByCount retains sort stability because std::list::sort also
  performs stable sort.

- merge builds another array and move it back to ValueData to avoid a
  potential quadratic behavior with std::vector::insert into the
  middle of a vector.

>From 13297fbe156d5a02596bfa08f92ead2517e7d21a Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Tue, 11 Jun 2024 11:22:14 -0700
Subject: [PATCH] [ProfileData] Use std::vector for ValueData (NFC)

This patch changes the type of ValueData to
std::vector<InstrProfValueData> so that, in a follow-up patch, we can
teach getValueForSite to return ArrayRef<InstrProfValueData>.

Currently, a typical traversal over the value data looks like:

  uint32_t NV = Func.getNumValueDataForSite(VK, I);
  std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, I);
  for (uint32_t V = 0; V < NV; V++)
    Do something with VD[V].Value and/or VD[V].Count;

Note that we need to call getNumValueDataForSite and getValueForSite
separately.  If getValueForSite returns ArrayRef<InstrProfValueData>
in the future, then we'll be able to do something like:

  for (const auto &V : Func.getValueForSite(VK, I))
    Do something with V.Value and/or V.Count;

If ArrayRef<InstrProfValueData> directly points to ValueData, then
getValueForSite won't need to allocate memory with std::make_unique.

Now, switching to std::vector requires us to update several places:

- sortByTargetValues switches to llvm::sort because we don't need to
  worry about sort stability.

- sortByCount retains sort stability because std::list::sort also
  performs stable sort.

- merge builds another array and move it back to ValueData to avoid a
  potential quadratic behavior with std::vector::insert into the
  middle of a vector.
---
 llvm/include/llvm/ProfileData/InstrProf.h | 16 ++++++++--------
 llvm/lib/ProfileData/InstrProf.cpp        | 11 +++++++++--
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index dae2caf0181e4..69589feb26b52 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -795,7 +795,7 @@ struct OverlapFuncFilters {
 
 struct InstrProfValueSiteRecord {
   /// Value profiling data pairs at a given value site.
-  std::list<InstrProfValueData> ValueData;
+  std::vector<InstrProfValueData> ValueData;
 
   InstrProfValueSiteRecord() = default;
   template <class InputIterator>
@@ -804,10 +804,10 @@ struct InstrProfValueSiteRecord {
 
   /// Sort ValueData ascending by Value
   void sortByTargetValues() {
-    ValueData.sort(
-        [](const InstrProfValueData &left, const InstrProfValueData &right) {
-          return left.Value < right.Value;
-        });
+    llvm::sort(ValueData,
+               [](const InstrProfValueData &L, const InstrProfValueData &R) {
+                 return L.Value < R.Value;
+               });
   }
   /// Sort ValueData Descending by Count
   inline void sortByCount();
@@ -1106,9 +1106,9 @@ void InstrProfRecord::reserveSites(uint32_t ValueKind, uint32_t NumValueSites) {
 #include "llvm/ProfileData/InstrProfData.inc"
 
 void InstrProfValueSiteRecord::sortByCount() {
-  ValueData.sort(
-      [](const InstrProfValueData &left, const InstrProfValueData &right) {
-        return left.Count > right.Count;
+  llvm::stable_sort(
+      ValueData, [](const InstrProfValueData &L, const InstrProfValueData &R) {
+        return L.Count > R.Count;
       });
   // Now truncate
   size_t max_s = INSTR_PROF_MAX_NUM_VAL_PER_SITE;
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index d707c07e6dc50..a3ab7d743d555 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -847,19 +847,26 @@ void InstrProfValueSiteRecord::merge(InstrProfValueSiteRecord &Input,
   Input.sortByTargetValues();
   auto I = ValueData.begin();
   auto IE = ValueData.end();
+  std::vector<InstrProfValueData> Merged;
+  Merged.reserve(std::max(ValueData.size(), Input.ValueData.size()));
   for (const InstrProfValueData &J : Input.ValueData) {
-    while (I != IE && I->Value < J.Value)
+    while (I != IE && I->Value < J.Value) {
+      Merged.push_back(*I);
       ++I;
+    }
     if (I != IE && I->Value == J.Value) {
       bool Overflowed;
       I->Count = SaturatingMultiplyAdd(J.Count, Weight, I->Count, &Overflowed);
       if (Overflowed)
         Warn(instrprof_error::counter_overflow);
+      Merged.push_back(*I);
       ++I;
       continue;
     }
-    ValueData.insert(I, J);
+    Merged.push_back(J);
   }
+  Merged.insert(Merged.end(), I, IE);
+  ValueData = std::move(Merged);
 }
 
 void InstrProfValueSiteRecord::scale(uint64_t N, uint64_t D,



More information about the llvm-commits mailing list