[llvm] [ProfileData] Teach getValueForSite to return ArrayRef<InstrProfValueData> (PR #95335)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 12 17:25:16 PDT 2024
https://github.com/kazutakahirata updated https://github.com/llvm/llvm-project/pull/95335
>From 84e56ba2c9b6707d218554a2021fd623dda681fc Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Wed, 12 Jun 2024 14:20:09 -0700
Subject: [PATCH 1/2] [ProfileData] Teach getValueForSite to return
ArrayRef<InstrProfValueData>
Without this patch, a typical traversal over the value data looks
like:
uint32_t NV = Func.getNumValueDataForSite(VK, S);
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
for (uint32_t V = 0; V < NV; V++)
Do something with VD[V].Value and/or VD[V].Count;
With this patch, getValueForSite returns ArrayRef<InstrProfValueData>,
so we can do:
for (const auto &V : Func.getValueForSite(VK, S))
Do something with V.Value and/or V.Count;
To accommodate the migration, this patch renames the existing
getValueForSite and its uses to getValueForSiteLegacy. This patch
silently switches to the new signature for unit tests that don't
require any change like:
auto VD0 = R->getValueForSite(IPVK_VTableTarget, 0);
EXPECT_EQ(VD0[0].Value, getCalleeAddress(vtable2));
EXPECT_EQ(VD0[0].Count, 7U);
where we receive the return value with auto and reference the array
contents.
Everything else uses getValueForSiteLegacy for now. I'm planning to
migrate to the new signature and remove getValueForSiteLegacy and
getNumValueDataForSite in follow-up patches.
---
llvm/include/llvm/ProfileData/InstrProf.h | 25 +++++++++----
llvm/lib/ProfileData/InstrProf.cpp | 7 ++--
llvm/lib/ProfileData/InstrProfWriter.cpp | 6 ++--
llvm/tools/llvm-profdata/llvm-profdata.cpp | 3 +-
llvm/unittests/ProfileData/InstrProfTest.cpp | 37 ++++++++++----------
5 files changed, 47 insertions(+), 31 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 817ad9550f652..45e29d8394856 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -870,13 +870,17 @@ struct InstrProfRecord {
uint32_t Site) const;
/// Return the array of profiled values at \p Site.
- inline std::unique_ptr<InstrProfValueData[]>
+ inline ArrayRef<InstrProfValueData>
getValueForSite(uint32_t ValueKind, uint32_t Site) const;
+ /// Return the array of profiled values at \p Site.
+ inline std::unique_ptr<InstrProfValueData[]>
+ getValueForSiteLegacy(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.
- inline void getValueForSite(InstrProfValueData Dest[], uint32_t ValueKind,
- uint32_t Site) const;
+ inline void getValueForSiteLegacy(InstrProfValueData Dest[],
+ uint32_t ValueKind, uint32_t Site) const;
/// Reserve space for NumValueSites sites.
inline void reserveSites(uint32_t ValueKind, uint32_t NumValueSites);
@@ -1060,20 +1064,27 @@ uint32_t InstrProfRecord::getNumValueDataForSite(uint32_t ValueKind,
return getValueSitesForKind(ValueKind)[Site].ValueData.size();
}
-std::unique_ptr<InstrProfValueData[]>
+ArrayRef<InstrProfValueData>
InstrProfRecord::getValueForSite(uint32_t ValueKind, uint32_t Site) const {
+ return getValueSitesForKind(ValueKind)[Site].ValueData;
+}
+
+std::unique_ptr<InstrProfValueData[]>
+InstrProfRecord::getValueForSiteLegacy(uint32_t ValueKind,
+ uint32_t Site) const {
uint32_t N = getNumValueDataForSite(ValueKind, Site);
if (N == 0)
return std::unique_ptr<InstrProfValueData[]>(nullptr);
auto VD = std::make_unique<InstrProfValueData[]>(N);
- getValueForSite(VD.get(), ValueKind, Site);
+ getValueForSiteLegacy(VD.get(), ValueKind, Site);
return VD;
}
-void InstrProfRecord::getValueForSite(InstrProfValueData Dest[],
- uint32_t ValueKind, uint32_t Site) const {
+void InstrProfRecord::getValueForSiteLegacy(InstrProfValueData Dest[],
+ uint32_t ValueKind,
+ uint32_t Site) const {
uint32_t I = 0;
for (auto V : getValueSitesForKind(ValueKind)[Site].ValueData) {
Dest[I].Value = V.Value;
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 184e2c86d6584..a77f932195203 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -732,7 +732,7 @@ void InstrProfRecord::accumulateCounts(CountSumOrPercent &Sum) const {
uint32_t NumValueSites = getNumValueSites(VK);
for (size_t I = 0; I < NumValueSites; ++I) {
uint32_t NV = getNumValueDataForSite(VK, I);
- std::unique_ptr<InstrProfValueData[]> VD = getValueForSite(VK, I);
+ std::unique_ptr<InstrProfValueData[]> VD = getValueForSiteLegacy(VK, I);
for (uint32_t V = 0; V < NV; V++)
KindSum += VD[V].Count;
}
@@ -1095,7 +1095,8 @@ uint32_t getNumValueDataForSiteInstrProf(const void *R, uint32_t VK,
void getValueForSiteInstrProf(const void *R, InstrProfValueData *Dst,
uint32_t K, uint32_t S) {
- reinterpret_cast<const InstrProfRecord *>(R)->getValueForSite(Dst, K, S);
+ reinterpret_cast<const InstrProfRecord *>(R)->getValueForSiteLegacy(Dst, K,
+ S);
}
ValueProfData *allocValueProfDataInstrProf(size_t TotalSizeInBytes) {
@@ -1279,7 +1280,7 @@ void annotateValueSite(Module &M, Instruction &Inst,
return;
std::unique_ptr<InstrProfValueData[]> VD =
- InstrProfR.getValueForSite(ValueKind, SiteIdx);
+ InstrProfR.getValueForSiteLegacy(ValueKind, SiteIdx);
ArrayRef<InstrProfValueData> VDs(VD.get(), NV);
uint64_t Sum = 0;
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 1a9add109a360..7263950273394 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -1040,7 +1040,8 @@ Error InstrProfWriter::validateRecord(const InstrProfRecord &Func) {
continue;
for (uint32_t S = 0; S < NS; S++) {
uint32_t ND = Func.getNumValueDataForSite(VK, S);
- std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
+ std::unique_ptr<InstrProfValueData[]> VD =
+ Func.getValueForSiteLegacy(VK, S);
DenseSet<uint64_t> SeenValues;
for (uint32_t I = 0; I < ND; I++)
if ((VK != IPVK_IndirectCallTarget && VK != IPVK_VTableTarget) &&
@@ -1090,7 +1091,8 @@ void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
for (uint32_t S = 0; S < NS; S++) {
uint32_t ND = Func.getNumValueDataForSite(VK, S);
OS << ND << "\n";
- std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
+ std::unique_ptr<InstrProfValueData[]> VD =
+ Func.getValueForSiteLegacy(VK, S);
for (uint32_t I = 0; I < ND; I++) {
if (VK == IPVK_IndirectCallTarget || VK == IPVK_VTableTarget)
OS << Symtab.getFuncOrVarNameIfDefined(VD[I].Value) << ":"
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index fae6d1e989ab5..0c67bace60561 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -2696,7 +2696,8 @@ static void traverseAllValueSites(const InstrProfRecord &Func, uint32_t 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);
+ std::unique_ptr<InstrProfValueData[]> VD =
+ Func.getValueForSiteLegacy(VK, I);
Stats.TotalNumValues += NV;
if (NV) {
Stats.TotalNumValueSitesWithValueProfile++;
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index 8acb0fa0c717a..8573a8d00c4e4 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1135,7 +1135,7 @@ TEST_P(MaybeSparseInstrProfTest, icall_and_vtable_data_merge) {
EXPECT_STREQ((const char *)VD[3].Value, "callee1");
EXPECT_EQ(VD[3].Count, 1U);
- auto VD_2(R->getValueForSite(IPVK_IndirectCallTarget, 2));
+ auto VD_2(R->getValueForSiteLegacy(IPVK_IndirectCallTarget, 2));
EXPECT_STREQ((const char *)VD_2[0].Value, "callee3");
EXPECT_EQ(VD_2[0].Count, 6U);
EXPECT_STREQ((const char *)VD_2[1].Value, "callee4");
@@ -1145,13 +1145,13 @@ TEST_P(MaybeSparseInstrProfTest, icall_and_vtable_data_merge) {
EXPECT_STREQ((const char *)VD_2[3].Value, "callee1");
EXPECT_EQ(VD_2[3].Count, 1U);
- auto VD_3(R->getValueForSite(IPVK_IndirectCallTarget, 3));
+ auto VD_3(R->getValueForSiteLegacy(IPVK_IndirectCallTarget, 3));
EXPECT_STREQ((const char *)VD_3[0].Value, "callee8");
EXPECT_EQ(VD_3[0].Count, 2U);
EXPECT_STREQ((const char *)VD_3[1].Value, "callee7");
EXPECT_EQ(VD_3[1].Count, 1U);
- auto VD_4(R->getValueForSite(IPVK_IndirectCallTarget, 4));
+ auto VD_4(R->getValueForSiteLegacy(IPVK_IndirectCallTarget, 4));
EXPECT_STREQ((const char *)VD_4[0].Value, "callee3");
EXPECT_EQ(VD_4[0].Count, 6U);
EXPECT_STREQ((const char *)VD_4[1].Value, "callee2");
@@ -1256,7 +1256,7 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_saturation) {
ASSERT_TRUE(bool(ReadRecord2));
ASSERT_EQ(1U, ReadRecord2->getNumValueSites(ValueKind));
std::unique_ptr<InstrProfValueData[]> VD =
- ReadRecord2->getValueForSite(ValueKind, 0);
+ ReadRecord2->getValueForSiteLegacy(ValueKind, 0);
EXPECT_EQ(ProfiledValue, VD[0].Value);
EXPECT_EQ(MaxValCount, VD[0].Count);
}
@@ -1300,7 +1300,8 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_site_trunc) {
Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
ASSERT_THAT_ERROR(R.takeError(), Succeeded());
- std::unique_ptr<InstrProfValueData[]> VD(R->getValueForSite(ValueKind, 0));
+ std::unique_ptr<InstrProfValueData[]> VD(
+ R->getValueForSiteLegacy(ValueKind, 0));
ASSERT_EQ(2U, R->getNumValueSites(ValueKind));
EXPECT_EQ(255U, R->getNumValueDataForSite(ValueKind, 0));
for (unsigned I = 0; I < 255; I++) {
@@ -1394,7 +1395,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
};
std::unique_ptr<InstrProfValueData[]> VD_0(
- Record.getValueForSite(IPVK_IndirectCallTarget, 0));
+ Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 0));
llvm::sort(&VD_0[0], &VD_0[5], Cmp);
EXPECT_STREQ((const char *)VD_0[0].Value, "callee2");
EXPECT_EQ(1000U, VD_0[0].Count);
@@ -1408,7 +1409,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_EQ(100U, VD_0[4].Count);
std::unique_ptr<InstrProfValueData[]> VD_1(
- Record.getValueForSite(IPVK_IndirectCallTarget, 1));
+ Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 1));
llvm::sort(&VD_1[0], &VD_1[4], Cmp);
EXPECT_STREQ((const char *)VD_1[0].Value, "callee2");
EXPECT_EQ(VD_1[0].Count, 2500U);
@@ -1420,7 +1421,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_EQ(VD_1[3].Count, 800U);
std::unique_ptr<InstrProfValueData[]> VD_2(
- Record.getValueForSite(IPVK_IndirectCallTarget, 2));
+ Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 2));
llvm::sort(&VD_2[0], &VD_2[3], Cmp);
EXPECT_STREQ((const char *)VD_2[0].Value, "callee4");
EXPECT_EQ(VD_2[0].Count, 5500U);
@@ -1430,7 +1431,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_EQ(VD_2[2].Count, 800U);
std::unique_ptr<InstrProfValueData[]> VD_3(
- Record.getValueForSite(IPVK_IndirectCallTarget, 3));
+ Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 3));
llvm::sort(&VD_3[0], &VD_3[2], Cmp);
EXPECT_STREQ((const char *)VD_3[0].Value, "callee3");
EXPECT_EQ(VD_3[0].Count, 2000U);
@@ -1443,7 +1444,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 2), 3U);
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 3), 2U);
- auto VD0(Record.getValueForSite(IPVK_VTableTarget, 0));
+ auto VD0(Record.getValueForSiteLegacy(IPVK_VTableTarget, 0));
llvm::sort(&VD0[0], &VD0[5], Cmp);
EXPECT_EQ(VD0[0].Value, getCalleeAddress(vtable2));
EXPECT_EQ(VD0[0].Count, 1000U);
@@ -1456,7 +1457,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_EQ(VD0[4].Value, getCalleeAddress(vtable5));
EXPECT_EQ(VD0[4].Count, 100U);
- auto VD1(Record.getValueForSite(IPVK_VTableTarget, 1));
+ auto VD1(Record.getValueForSiteLegacy(IPVK_VTableTarget, 1));
llvm::sort(&VD1[0], &VD1[4], Cmp);
EXPECT_EQ(VD1[0].Value, getCalleeAddress(vtable2));
EXPECT_EQ(VD1[0].Count, 2500U);
@@ -1467,7 +1468,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_EQ(VD1[3].Value, getCalleeAddress(vtable5));
EXPECT_EQ(VD1[3].Count, 800U);
- auto VD2(Record.getValueForSite(IPVK_VTableTarget, 2));
+ auto VD2(Record.getValueForSiteLegacy(IPVK_VTableTarget, 2));
llvm::sort(&VD2[0], &VD2[3], Cmp);
EXPECT_EQ(VD2[0].Value, getCalleeAddress(vtable4));
EXPECT_EQ(VD2[0].Count, 5500U);
@@ -1476,7 +1477,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_EQ(VD2[2].Value, getCalleeAddress(vtable6));
EXPECT_EQ(VD2[2].Count, 800U);
- auto VD3(Record.getValueForSite(IPVK_VTableTarget, 3));
+ auto VD3(Record.getValueForSiteLegacy(IPVK_VTableTarget, 3));
llvm::sort(&VD3[0], &VD3[2], Cmp);
EXPECT_EQ(VD3[0].Value, getCalleeAddress(vtable3));
EXPECT_EQ(VD3[0].Count, 2000U);
@@ -1538,7 +1539,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
auto Cmp = [](const InstrProfValueData &VD1, const InstrProfValueData &VD2) {
return VD1.Count > VD2.Count;
};
- auto VD_0(Record.getValueForSite(IPVK_IndirectCallTarget, 0));
+ auto VD_0(Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 0));
llvm::sort(&VD_0[0], &VD_0[5], Cmp);
ASSERT_EQ(VD_0[0].Value, 0x2000ULL);
ASSERT_EQ(VD_0[0].Count, 1000U);
@@ -1555,7 +1556,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The first vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 0));
+ auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 0));
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 0), 5U);
llvm::sort(&VD[0], &VD[5], Cmp);
EXPECT_EQ(VD[0].Count, 1000U);
@@ -1574,7 +1575,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The second vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 1));
+ auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 1));
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 1), 4U);
llvm::sort(&VD[0], &VD[4], Cmp);
EXPECT_EQ(VD[0].Value, MD5Hash("vtable2"));
@@ -1591,7 +1592,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The third vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 2));
+ auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 2));
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 2), 3U);
llvm::sort(&VD[0], &VD[3], Cmp);
EXPECT_EQ(VD[0].Count, 5500U);
@@ -1605,7 +1606,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The fourth vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 3));
+ auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 3));
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 3), 2U);
llvm::sort(&VD[0], &VD[2], Cmp);
EXPECT_EQ(VD[0].Count, 2000U);
>From 78d153b7328b17b261b3302b10b7d3ef0ea59130 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Wed, 12 Jun 2024 17:24:31 -0700
Subject: [PATCH 2/2] Fix formatting.
---
llvm/include/llvm/ProfileData/InstrProf.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 45e29d8394856..e2a014b4fbbed 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -870,8 +870,8 @@ struct InstrProfRecord {
uint32_t Site) const;
/// Return the array of profiled values at \p Site.
- inline ArrayRef<InstrProfValueData>
- getValueForSite(uint32_t ValueKind, uint32_t Site) const;
+ inline ArrayRef<InstrProfValueData> getValueForSite(uint32_t ValueKind,
+ uint32_t Site) const;
/// Return the array of profiled values at \p Site.
inline std::unique_ptr<InstrProfValueData[]>
More information about the llvm-commits
mailing list