Skip to content

Commit 11c7568

Browse files
kazutakahiratakazutakahirata
committed
[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.
1 parent 48f8d95 commit 11c7568

File tree

5 files changed

+47
-31
lines changed

5 files changed

+47
-31
lines changed

‎llvm/include/llvm/ProfileData/InstrProf.h

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -870,13 +870,17 @@ struct InstrProfRecord {
870870
uint32_t Site) const;
871871

872872
/// Return the array of profiled values at \p Site.
873-
inline std::unique_ptr<InstrProfValueData[]>
873+
inline ArrayRef<InstrProfValueData>
874874
getValueForSite(uint32_t ValueKind, uint32_t Site) const;
875875

876+
/// Return the array of profiled values at \p Site.
877+
inline std::unique_ptr<InstrProfValueData[]>
878+
getValueForSiteLegacy(uint32_t ValueKind, uint32_t Site) const;
879+
876880
/// Get the target value/counts of kind \p ValueKind collected at site
877881
/// \p Site and store the result in array \p Dest.
878-
inline void getValueForSite(InstrProfValueData Dest[], uint32_t ValueKind,
879-
uint32_t Site) const;
882+
inline void getValueForSiteLegacy(InstrProfValueData Dest[],
883+
uint32_t ValueKind, uint32_t Site) const;
880884

881885
/// Reserve space for NumValueSites sites.
882886
inline void reserveSites(uint32_t ValueKind, uint32_t NumValueSites);
@@ -1060,20 +1064,27 @@ uint32_t InstrProfRecord::getNumValueDataForSite(uint32_t ValueKind,
10601064
return getValueSitesForKind(ValueKind)[Site].ValueData.size();
10611065
}
10621066

1063-
std::unique_ptr<InstrProfValueData[]>
1067+
ArrayRef<InstrProfValueData>
10641068
InstrProfRecord::getValueForSite(uint32_t ValueKind, uint32_t Site) const {
1069+
return getValueSitesForKind(ValueKind)[Site].ValueData;
1070+
}
1071+
1072+
std::unique_ptr<InstrProfValueData[]>
1073+
InstrProfRecord::getValueForSiteLegacy(uint32_t ValueKind,
1074+
uint32_t Site) const {
10651075
uint32_t N = getNumValueDataForSite(ValueKind, Site);
10661076
if (N == 0)
10671077
return std::unique_ptr<InstrProfValueData[]>(nullptr);
10681078

10691079
auto VD = std::make_unique<InstrProfValueData[]>(N);
1070-
getValueForSite(VD.get(), ValueKind, Site);
1080+
getValueForSiteLegacy(VD.get(), ValueKind, Site);
10711081

10721082
return VD;
10731083
}
10741084

1075-
void InstrProfRecord::getValueForSite(InstrProfValueData Dest[],
1076-
uint32_t ValueKind, uint32_t Site) const {
1085+
void InstrProfRecord::getValueForSiteLegacy(InstrProfValueData Dest[],
1086+
uint32_t ValueKind,
1087+
uint32_t Site) const {
10771088
uint32_t I = 0;
10781089
for (auto V : getValueSitesForKind(ValueKind)[Site].ValueData) {
10791090
Dest[I].Value = V.Value;

‎llvm/lib/ProfileData/InstrProf.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ void InstrProfRecord::accumulateCounts(CountSumOrPercent &Sum) const {
732732
uint32_t NumValueSites = getNumValueSites(VK);
733733
for (size_t I = 0; I < NumValueSites; ++I) {
734734
uint32_t NV = getNumValueDataForSite(VK, I);
735-
std::unique_ptr<InstrProfValueData[]> VD = getValueForSite(VK, I);
735+
std::unique_ptr<InstrProfValueData[]> VD = getValueForSiteLegacy(VK, I);
736736
for (uint32_t V = 0; V < NV; V++)
737737
KindSum += VD[V].Count;
738738
}
@@ -1095,7 +1095,8 @@ uint32_t getNumValueDataForSiteInstrProf(const void *R, uint32_t VK,
10951095

10961096
void getValueForSiteInstrProf(const void *R, InstrProfValueData *Dst,
10971097
uint32_t K, uint32_t S) {
1098-
reinterpret_cast<const InstrProfRecord *>(R)->getValueForSite(Dst, K, S);
1098+
reinterpret_cast<const InstrProfRecord *>(R)->getValueForSiteLegacy(Dst, K,
1099+
S);
10991100
}
11001101

11011102
ValueProfData *allocValueProfDataInstrProf(size_t TotalSizeInBytes) {
@@ -1279,7 +1280,7 @@ void annotateValueSite(Module &M, Instruction &Inst,
12791280
return;
12801281

12811282
std::unique_ptr<InstrProfValueData[]> VD =
1282-
InstrProfR.getValueForSite(ValueKind, SiteIdx);
1283+
InstrProfR.getValueForSiteLegacy(ValueKind, SiteIdx);
12831284

12841285
ArrayRef<InstrProfValueData> VDs(VD.get(), NV);
12851286
uint64_t Sum = 0;

‎llvm/lib/ProfileData/InstrProfWriter.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,8 @@ Error InstrProfWriter::validateRecord(const InstrProfRecord &Func) {
10401040
continue;
10411041
for (uint32_t S = 0; S < NS; S++) {
10421042
uint32_t ND = Func.getNumValueDataForSite(VK, S);
1043-
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
1043+
std::unique_ptr<InstrProfValueData[]> VD =
1044+
Func.getValueForSiteLegacy(VK, S);
10441045
DenseSet<uint64_t> SeenValues;
10451046
for (uint32_t I = 0; I < ND; I++)
10461047
if ((VK != IPVK_IndirectCallTarget && VK != IPVK_VTableTarget) &&
@@ -1090,7 +1091,8 @@ void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
10901091
for (uint32_t S = 0; S < NS; S++) {
10911092
uint32_t ND = Func.getNumValueDataForSite(VK, S);
10921093
OS << ND << "\n";
1093-
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
1094+
std::unique_ptr<InstrProfValueData[]> VD =
1095+
Func.getValueForSiteLegacy(VK, S);
10941096
for (uint32_t I = 0; I < ND; I++) {
10951097
if (VK == IPVK_IndirectCallTarget || VK == IPVK_VTableTarget)
10961098
OS << Symtab.getFuncOrVarNameIfDefined(VD[I].Value) << ":"

‎llvm/tools/llvm-profdata/llvm-profdata.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2696,7 +2696,8 @@ static void traverseAllValueSites(const InstrProfRecord &Func, uint32_t VK,
26962696
Stats.TotalNumValueSites += NS;
26972697
for (size_t I = 0; I < NS; ++I) {
26982698
uint32_t NV = Func.getNumValueDataForSite(VK, I);
2699-
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, I);
2699+
std::unique_ptr<InstrProfValueData[]> VD =
2700+
Func.getValueForSiteLegacy(VK, I);
27002701
Stats.TotalNumValues += NV;
27012702
if (NV) {
27022703
Stats.TotalNumValueSitesWithValueProfile++;

‎llvm/unittests/ProfileData/InstrProfTest.cpp

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,7 +1135,7 @@ TEST_P(MaybeSparseInstrProfTest, icall_and_vtable_data_merge) {
11351135
EXPECT_STREQ((const char *)VD[3].Value, "callee1");
11361136
EXPECT_EQ(VD[3].Count, 1U);
11371137

1138-
auto VD_2(R->getValueForSite(IPVK_IndirectCallTarget, 2));
1138+
auto VD_2(R->getValueForSiteLegacy(IPVK_IndirectCallTarget, 2));
11391139
EXPECT_STREQ((const char *)VD_2[0].Value, "callee3");
11401140
EXPECT_EQ(VD_2[0].Count, 6U);
11411141
EXPECT_STREQ((const char *)VD_2[1].Value, "callee4");
@@ -1145,13 +1145,13 @@ TEST_P(MaybeSparseInstrProfTest, icall_and_vtable_data_merge) {
11451145
EXPECT_STREQ((const char *)VD_2[3].Value, "callee1");
11461146
EXPECT_EQ(VD_2[3].Count, 1U);
11471147

1148-
auto VD_3(R->getValueForSite(IPVK_IndirectCallTarget, 3));
1148+
auto VD_3(R->getValueForSiteLegacy(IPVK_IndirectCallTarget, 3));
11491149
EXPECT_STREQ((const char *)VD_3[0].Value, "callee8");
11501150
EXPECT_EQ(VD_3[0].Count, 2U);
11511151
EXPECT_STREQ((const char *)VD_3[1].Value, "callee7");
11521152
EXPECT_EQ(VD_3[1].Count, 1U);
11531153

1154-
auto VD_4(R->getValueForSite(IPVK_IndirectCallTarget, 4));
1154+
auto VD_4(R->getValueForSiteLegacy(IPVK_IndirectCallTarget, 4));
11551155
EXPECT_STREQ((const char *)VD_4[0].Value, "callee3");
11561156
EXPECT_EQ(VD_4[0].Count, 6U);
11571157
EXPECT_STREQ((const char *)VD_4[1].Value, "callee2");
@@ -1256,7 +1256,7 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_saturation) {
12561256
ASSERT_TRUE(bool(ReadRecord2));
12571257
ASSERT_EQ(1U, ReadRecord2->getNumValueSites(ValueKind));
12581258
std::unique_ptr<InstrProfValueData[]> VD =
1259-
ReadRecord2->getValueForSite(ValueKind, 0);
1259+
ReadRecord2->getValueForSiteLegacy(ValueKind, 0);
12601260
EXPECT_EQ(ProfiledValue, VD[0].Value);
12611261
EXPECT_EQ(MaxValCount, VD[0].Count);
12621262
}
@@ -1300,7 +1300,8 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_site_trunc) {
13001300

13011301
Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
13021302
ASSERT_THAT_ERROR(R.takeError(), Succeeded());
1303-
std::unique_ptr<InstrProfValueData[]> VD(R->getValueForSite(ValueKind, 0));
1303+
std::unique_ptr<InstrProfValueData[]> VD(
1304+
R->getValueForSiteLegacy(ValueKind, 0));
13041305
ASSERT_EQ(2U, R->getNumValueSites(ValueKind));
13051306
EXPECT_EQ(255U, R->getNumValueDataForSite(ValueKind, 0));
13061307
for (unsigned I = 0; I < 255; I++) {
@@ -1394,7 +1395,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
13941395
};
13951396

13961397
std::unique_ptr<InstrProfValueData[]> VD_0(
1397-
Record.getValueForSite(IPVK_IndirectCallTarget, 0));
1398+
Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 0));
13981399
llvm::sort(&VD_0[0], &VD_0[5], Cmp);
13991400
EXPECT_STREQ((const char *)VD_0[0].Value, "callee2");
14001401
EXPECT_EQ(1000U, VD_0[0].Count);
@@ -1408,7 +1409,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
14081409
EXPECT_EQ(100U, VD_0[4].Count);
14091410

14101411
std::unique_ptr<InstrProfValueData[]> VD_1(
1411-
Record.getValueForSite(IPVK_IndirectCallTarget, 1));
1412+
Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 1));
14121413
llvm::sort(&VD_1[0], &VD_1[4], Cmp);
14131414
EXPECT_STREQ((const char *)VD_1[0].Value, "callee2");
14141415
EXPECT_EQ(VD_1[0].Count, 2500U);
@@ -1420,7 +1421,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
14201421
EXPECT_EQ(VD_1[3].Count, 800U);
14211422

14221423
std::unique_ptr<InstrProfValueData[]> VD_2(
1423-
Record.getValueForSite(IPVK_IndirectCallTarget, 2));
1424+
Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 2));
14241425
llvm::sort(&VD_2[0], &VD_2[3], Cmp);
14251426
EXPECT_STREQ((const char *)VD_2[0].Value, "callee4");
14261427
EXPECT_EQ(VD_2[0].Count, 5500U);
@@ -1430,7 +1431,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
14301431
EXPECT_EQ(VD_2[2].Count, 800U);
14311432

14321433
std::unique_ptr<InstrProfValueData[]> VD_3(
1433-
Record.getValueForSite(IPVK_IndirectCallTarget, 3));
1434+
Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 3));
14341435
llvm::sort(&VD_3[0], &VD_3[2], Cmp);
14351436
EXPECT_STREQ((const char *)VD_3[0].Value, "callee3");
14361437
EXPECT_EQ(VD_3[0].Count, 2000U);
@@ -1443,7 +1444,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
14431444
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 2), 3U);
14441445
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 3), 2U);
14451446

1446-
auto VD0(Record.getValueForSite(IPVK_VTableTarget, 0));
1447+
auto VD0(Record.getValueForSiteLegacy(IPVK_VTableTarget, 0));
14471448
llvm::sort(&VD0[0], &VD0[5], Cmp);
14481449
EXPECT_EQ(VD0[0].Value, getCalleeAddress(vtable2));
14491450
EXPECT_EQ(VD0[0].Count, 1000U);
@@ -1456,7 +1457,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
14561457
EXPECT_EQ(VD0[4].Value, getCalleeAddress(vtable5));
14571458
EXPECT_EQ(VD0[4].Count, 100U);
14581459

1459-
auto VD1(Record.getValueForSite(IPVK_VTableTarget, 1));
1460+
auto VD1(Record.getValueForSiteLegacy(IPVK_VTableTarget, 1));
14601461
llvm::sort(&VD1[0], &VD1[4], Cmp);
14611462
EXPECT_EQ(VD1[0].Value, getCalleeAddress(vtable2));
14621463
EXPECT_EQ(VD1[0].Count, 2500U);
@@ -1467,7 +1468,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
14671468
EXPECT_EQ(VD1[3].Value, getCalleeAddress(vtable5));
14681469
EXPECT_EQ(VD1[3].Count, 800U);
14691470

1470-
auto VD2(Record.getValueForSite(IPVK_VTableTarget, 2));
1471+
auto VD2(Record.getValueForSiteLegacy(IPVK_VTableTarget, 2));
14711472
llvm::sort(&VD2[0], &VD2[3], Cmp);
14721473
EXPECT_EQ(VD2[0].Value, getCalleeAddress(vtable4));
14731474
EXPECT_EQ(VD2[0].Count, 5500U);
@@ -1476,7 +1477,7 @@ TEST(ValueProfileReadWriteTest, value_prof_data_read_write) {
14761477
EXPECT_EQ(VD2[2].Value, getCalleeAddress(vtable6));
14771478
EXPECT_EQ(VD2[2].Count, 800U);
14781479

1479-
auto VD3(Record.getValueForSite(IPVK_VTableTarget, 3));
1480+
auto VD3(Record.getValueForSiteLegacy(IPVK_VTableTarget, 3));
14801481
llvm::sort(&VD3[0], &VD3[2], Cmp);
14811482
EXPECT_EQ(VD3[0].Value, getCalleeAddress(vtable3));
14821483
EXPECT_EQ(VD3[0].Count, 2000U);
@@ -1538,7 +1539,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
15381539
auto Cmp = [](const InstrProfValueData &VD1, const InstrProfValueData &VD2) {
15391540
return VD1.Count > VD2.Count;
15401541
};
1541-
auto VD_0(Record.getValueForSite(IPVK_IndirectCallTarget, 0));
1542+
auto VD_0(Record.getValueForSiteLegacy(IPVK_IndirectCallTarget, 0));
15421543
llvm::sort(&VD_0[0], &VD_0[5], Cmp);
15431544
ASSERT_EQ(VD_0[0].Value, 0x2000ULL);
15441545
ASSERT_EQ(VD_0[0].Count, 1000U);
@@ -1555,7 +1556,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
15551556

15561557
{
15571558
// The first vtable site.
1558-
auto VD(Record.getValueForSite(IPVK_VTableTarget, 0));
1559+
auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 0));
15591560
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 0), 5U);
15601561
llvm::sort(&VD[0], &VD[5], Cmp);
15611562
EXPECT_EQ(VD[0].Count, 1000U);
@@ -1574,7 +1575,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
15741575

15751576
{
15761577
// The second vtable site.
1577-
auto VD(Record.getValueForSite(IPVK_VTableTarget, 1));
1578+
auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 1));
15781579
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 1), 4U);
15791580
llvm::sort(&VD[0], &VD[4], Cmp);
15801581
EXPECT_EQ(VD[0].Value, MD5Hash("vtable2"));
@@ -1591,7 +1592,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
15911592

15921593
{
15931594
// The third vtable site.
1594-
auto VD(Record.getValueForSite(IPVK_VTableTarget, 2));
1595+
auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 2));
15951596
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 2), 3U);
15961597
llvm::sort(&VD[0], &VD[3], Cmp);
15971598
EXPECT_EQ(VD[0].Count, 5500U);
@@ -1605,7 +1606,7 @@ TEST(ValueProfileReadWriteTest, symtab_mapping) {
16051606

16061607
{
16071608
// The fourth vtable site.
1608-
auto VD(Record.getValueForSite(IPVK_VTableTarget, 3));
1609+
auto VD(Record.getValueForSiteLegacy(IPVK_VTableTarget, 3));
16091610
ASSERT_EQ(Record.getNumValueDataForSite(IPVK_VTableTarget, 3), 2U);
16101611
llvm::sort(&VD[0], &VD[2], Cmp);
16111612
EXPECT_EQ(VD[0].Count, 2000U);

0 commit comments

Comments
 (0)