[ProfileData] Migrate to getValueArrayForSite #95493
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch migrates uses of getValueForSite to getValueArrayForSite. Each hunk is self-contained, meaning that each one can be applied independently of the others. In the unit test, there are cases where the array length check is performed a lot earlier than the array content check. For now, I'm leaving the length checks where they are. I'll consider moving them when I migrate uses of getNumValueDataForSite to getValueArrayForSite in a follow-up patch.
|
@llvm/pr-subscribers-pgo Author: Kazu Hirata (kazutakahirata) ChangesThis patch migrates uses of getValueForSite to getValueArrayForSite. In the unit test, there are cases where the array length check is Full diff: https://github.com/llvm/llvm-project/pull/95493.diff 3 Files Affected:
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 184e2c86d6584..a0662a5976bbe 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -731,10 +731,8 @@ void InstrProfRecord::accumulateCounts(CountSumOrPercent &Sum) const {
uint64_t KindSum = 0;
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);
- for (uint32_t V = 0; V < NV; V++)
- KindSum += VD[V].Count;
+ for (const auto &V : getValueArrayForSite(VK, I))
+ KindSum += V.Count;
}
Sum.ValueCounts[VK] += KindSum;
}
@@ -1089,13 +1087,14 @@ uint32_t getNumValueDataInstrProf(const void *Record, uint32_t VKind) {
uint32_t getNumValueDataForSiteInstrProf(const void *R, uint32_t VK,
uint32_t S) {
- return reinterpret_cast<const InstrProfRecord *>(R)
- ->getNumValueDataForSite(VK, S);
+ const auto *IPR = reinterpret_cast<const InstrProfRecord *>(R);
+ return IPR->getValueArrayForSite(VK, S).size();
}
void getValueForSiteInstrProf(const void *R, InstrProfValueData *Dst,
uint32_t K, uint32_t S) {
- reinterpret_cast<const InstrProfRecord *>(R)->getValueForSite(Dst, K, S);
+ const auto *IPR = reinterpret_cast<const InstrProfRecord *>(R);
+ llvm::copy(IPR->getValueArrayForSite(K, S), Dst);
}
ValueProfData *allocValueProfDataInstrProf(size_t TotalSizeInBytes) {
@@ -1274,14 +1273,9 @@ void annotateValueSite(Module &M, Instruction &Inst,
const InstrProfRecord &InstrProfR,
InstrProfValueKind ValueKind, uint32_t SiteIdx,
uint32_t MaxMDCount) {
- uint32_t NV = InstrProfR.getNumValueDataForSite(ValueKind, SiteIdx);
- if (!NV)
+ auto VDs = InstrProfR.getValueArrayForSite(ValueKind, SiteIdx);
+ if (VDs.empty())
return;
-
- std::unique_ptr<InstrProfValueData[]> VD =
- InstrProfR.getValueForSite(ValueKind, SiteIdx);
-
- ArrayRef<InstrProfValueData> VDs(VD.get(), NV);
uint64_t Sum = 0;
for (const InstrProfValueData &V : VDs)
Sum = SaturatingAdd(Sum, V.Count);
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 1a9add109a360..7cf4704a79faa 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -1088,15 +1088,14 @@ void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
OS << "# ValueKind = " << ValueProfKindStr[VK] << ":\n" << VK << "\n";
OS << "# NumValueSites:\n" << NS << "\n";
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);
- for (uint32_t I = 0; I < ND; I++) {
+ auto VD = Func.getValueArrayForSite(VK, S);
+ OS << VD.size() << "\n";
+ for (const auto &V : VD) {
if (VK == IPVK_IndirectCallTarget || VK == IPVK_VTableTarget)
- OS << Symtab.getFuncOrVarNameIfDefined(VD[I].Value) << ":"
- << VD[I].Count << "\n";
+ OS << Symtab.getFuncOrVarNameIfDefined(V.Value) << ":" << V.Count
+ << "\n";
else
- OS << VD[I].Value << ":" << VD[I].Count << "\n";
+ OS << V.Value << ":" << V.Count << "\n";
}
}
}
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index 8a04281efeb50..dae5542290934 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -1392,9 +1392,9 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
return VD1.Count > VD2.Count;
};
- std::unique_ptr<InstrProfValueData[]> VD_0(
- Record.getValueForSite(IPVK_IndirectCallTarget, 0));
- llvm::sort(&VD_0[0], &VD_0[5], Cmp);
+ SmallVector<InstrProfValueData> VD_0(
+ Record.getValueArrayForSite(IPVK_IndirectCallTarget, 0));
+ llvm::sort(VD_0, Cmp);
EXPECT_STREQ((const char *)VD_0[0].Value, "callee2");
EXPECT_EQ(1000U, VD_0[0].Count);
EXPECT_STREQ((const char *)VD_0[1].Value, "callee3");
@@ -1406,9 +1406,9 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_STREQ((const char *)VD_0[4].Value, "callee5");
EXPECT_EQ(100U, VD_0[4].Count);
- std::unique_ptr<InstrProfValueData[]> VD_1(
- Record.getValueForSite(IPVK_IndirectCallTarget, 1));
- llvm::sort(&VD_1[0], &VD_1[4], Cmp);
+ SmallVector<InstrProfValueData> VD_1(
+ Record.getValueArrayForSite(IPVK_IndirectCallTarget, 1));
+ llvm::sort(VD_1, Cmp);
EXPECT_STREQ((const char *)VD_1[0].Value, "callee2");
EXPECT_EQ(VD_1[0].Count, 2500U);
EXPECT_STREQ((const char *)VD_1[1].Value, "callee1");
@@ -1418,9 +1418,9 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_STREQ((const char *)VD_1[3].Value, "callee5");
EXPECT_EQ(VD_1[3].Count, 800U);
- std::unique_ptr<InstrProfValueData[]> VD_2(
- Record.getValueForSite(IPVK_IndirectCallTarget, 2));
- llvm::sort(&VD_2[0], &VD_2[3], Cmp);
+ SmallVector<InstrProfValueData> VD_2(
+ Record.getValueArrayForSite(IPVK_IndirectCallTarget, 2));
+ llvm::sort(VD_2, Cmp);
EXPECT_STREQ((const char *)VD_2[0].Value, "callee4");
EXPECT_EQ(VD_2[0].Count, 5500U);
EXPECT_STREQ((const char *)VD_2[1].Value, "callee3");
@@ -1428,9 +1428,9 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
EXPECT_STREQ((const char *)VD_2[2].Value, "callee6");
EXPECT_EQ(VD_2[2].Count, 800U);
- std::unique_ptr<InstrProfValueData[]> VD_3(
- Record.getValueForSite(IPVK_IndirectCallTarget, 3));
- llvm::sort(&VD_3[0], &VD_3[2], Cmp);
+ SmallVector<InstrProfValueData> VD_3(
+ Record.getValueArrayForSite(IPVK_IndirectCallTarget, 3));
+ llvm::sort(VD_3, Cmp);
EXPECT_STREQ((const char *)VD_3[0].Value, "callee3");
EXPECT_EQ(VD_3[0].Count, 2000U);
EXPECT_STREQ((const char *)VD_3[1].Value, "callee2");
@@ -1442,8 +1442,9 @@ 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));
- llvm::sort(&VD0[0], &VD0[5], Cmp);
+ SmallVector<InstrProfValueData> VD0(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 0));
+ llvm::sort(VD0, Cmp);
EXPECT_EQ(VD0[0].Value, getCalleeAddress(vtable2));
EXPECT_EQ(VD0[0].Count, 1000U);
EXPECT_EQ(VD0[1].Value, getCalleeAddress(vtable3));
@@ -1455,8 +1456,9 @@ 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));
- llvm::sort(&VD1[0], &VD1[4], Cmp);
+ SmallVector<InstrProfValueData> VD1(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 1));
+ llvm::sort(VD1, Cmp);
EXPECT_EQ(VD1[0].Value, getCalleeAddress(vtable2));
EXPECT_EQ(VD1[0].Count, 2500U);
EXPECT_EQ(VD1[1].Value, getCalleeAddress(vtable1));
@@ -1466,8 +1468,9 @@ 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));
- llvm::sort(&VD2[0], &VD2[3], Cmp);
+ SmallVector<InstrProfValueData> VD2(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 2));
+ llvm::sort(VD2, Cmp);
EXPECT_EQ(VD2[0].Value, getCalleeAddress(vtable4));
EXPECT_EQ(VD2[0].Count, 5500U);
EXPECT_EQ(VD2[1].Value, getCalleeAddress(vtable3));
@@ -1475,8 +1478,9 @@ 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));
- llvm::sort(&VD3[0], &VD3[2], Cmp);
+ SmallVector<InstrProfValueData> VD3(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 3));
+ llvm::sort(VD3, Cmp);
EXPECT_EQ(VD3[0].Value, getCalleeAddress(vtable3));
EXPECT_EQ(VD3[0].Count, 2000U);
EXPECT_EQ(VD3[1].Value, getCalleeAddress(vtable2));
@@ -1537,8 +1541,9 @@ 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));
- llvm::sort(&VD_0[0], &VD_0[5], Cmp);
+ SmallVector<InstrProfValueData> VD_0(
+ Record.getValueArrayForSite(IPVK_IndirectCallTarget, 0));
+ llvm::sort(VD_0, Cmp);
ASSERT_EQ(VD_0[0].Value, 0x2000ULL);
ASSERT_EQ(VD_0[0].Count, 1000U);
ASSERT_EQ(VD_0[1].Value, 0x3000ULL);
@@ -1554,9 +1559,10 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The first vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 0));
- ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 0), 5U);
- llvm::sort(&VD[0], &VD[5], Cmp);
+ SmallVector<InstrProfValueData> VD(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 0));
+ ASSERT_THAT(VD, SizeIs(5));
+ llvm::sort(VD, Cmp);
EXPECT_EQ(VD[0].Count, 1000U);
EXPECT_EQ(VD[0].Value, MD5Hash("vtable2"));
EXPECT_EQ(VD[1].Count, 500U);
@@ -1573,9 +1579,10 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The second vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 1));
- ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 1), 4U);
- llvm::sort(&VD[0], &VD[4], Cmp);
+ SmallVector<InstrProfValueData> VD(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 1));
+ ASSERT_THAT(VD, SizeIs(4));
+ llvm::sort(VD, Cmp);
EXPECT_EQ(VD[0].Value, MD5Hash("vtable2"));
EXPECT_EQ(VD[0].Count, 2500U);
EXPECT_EQ(VD[1].Value, MD5Hash("vtable1"));
@@ -1590,9 +1597,10 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The third vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 2));
- ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 2), 3U);
- llvm::sort(&VD[0], &VD[3], Cmp);
+ SmallVector<InstrProfValueData> VD(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 2));
+ ASSERT_THAT(VD, SizeIs(3));
+ llvm::sort(VD, Cmp);
EXPECT_EQ(VD[0].Count, 5500U);
EXPECT_EQ(VD[0].Value, MD5Hash("vtable4"));
EXPECT_EQ(VD[1].Count, 1000U);
@@ -1604,9 +1612,10 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
{
// The fourth vtable site.
- auto VD(Record.getValueForSite(IPVK_VTableTarget, 3));
- ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 3), 2U);
- llvm::sort(&VD[0], &VD[2], Cmp);
+ SmallVector<InstrProfValueData> VD(
+ Record.getValueArrayForSite(IPVK_VTableTarget, 3));
+ ASSERT_THAT(VD, SizeIs(2));
+ llvm::sort(VD, Cmp);
EXPECT_EQ(VD[0].Count, 2000U);
EXPECT_EQ(VD[0].Value, MD5Hash("vtable3"));
EXPECT_EQ(VD[1].Count, 1800U);
|
david-xl
reviewed
Jun 14, 2024
david-xl
approved these changes
Jun 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch migrates uses of getValueForSite to getValueArrayForSite.
Each hunk is self-contained, meaning that each one can be applied
independently of the others.
In the unit test, there are cases where the array length check is
performed a lot earlier than the array content check. For now, I'm
leaving the length checks where they are. I'll consider moving them
when I migrate uses of getNumValueDataForSite to getValueArrayForSite
in a follow-up patch.