Skip to content

Commit 6c8ff4c

Browse files
kazutakahiratakazutakahirata
authored
[ProfileData] Take ArrayRef<InstrProfValueData> in addValueData (NFC) (llvm#97363)
This patch fixes another place in ProfileData where we have a pointer to an array of InstrProfValueData and its length separately. addValueData is a bit unique in that it remaps incoming values in place before adding them to ValueSites. AFAICT, no caller of addValueData uses updated incoming values. With this patch, we add value data to ValueSites first and then remaps values there. This way, we can take ArrayRef<InstrProfValueData> as a parameter.
1 parent 46307f1 commit 6c8ff4c

File tree

4 files changed

+57
-53
lines changed

4 files changed

+57
-53
lines changed

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -794,9 +794,8 @@ struct InstrProfValueSiteRecord {
794794
std::vector<InstrProfValueData> ValueData;
795795

796796
InstrProfValueSiteRecord() = default;
797-
template <class InputIterator>
798-
InstrProfValueSiteRecord(InputIterator F, InputIterator L)
799-
: ValueData(F, L) {}
797+
InstrProfValueSiteRecord(std::vector<InstrProfValueData> &&VD)
798+
: ValueData(VD) {}
800799

801800
/// Sort ValueData ascending by Value
802801
void sortByTargetValues() {
@@ -870,7 +869,7 @@ struct InstrProfRecord {
870869
/// Add ValueData for ValueKind at value Site. We do not support adding sites
871870
/// out of order. Site must go up from 0 one by one.
872871
void addValueData(uint32_t ValueKind, uint32_t Site,
873-
InstrProfValueData *VData, uint32_t N,
872+
ArrayRef<InstrProfValueData> VData,
874873
InstrProfSymtab *SymTab);
875874

876875
/// Merge the counts in \p Other into this one.

‎llvm/lib/ProfileData/InstrProf.cpp

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -998,18 +998,22 @@ uint64_t InstrProfRecord::remapValue(uint64_t Value, uint32_t ValueKind,
998998
}
999999

10001000
void InstrProfRecord::addValueData(uint32_t ValueKind, uint32_t Site,
1001-
InstrProfValueData *VData, uint32_t N,
1001+
ArrayRef<InstrProfValueData> VData,
10021002
InstrProfSymtab *ValueMap) {
1003-
for (uint32_t I = 0; I < N; I++) {
1004-
VData[I].Value = remapValue(VData[I].Value, ValueKind, ValueMap);
1003+
// Remap values.
1004+
std::vector<InstrProfValueData> RemappedVD;
1005+
RemappedVD.reserve(VData.size());
1006+
for (const auto &V : VData) {
1007+
uint64_t NewValue = remapValue(V.Value, ValueKind, ValueMap);
1008+
RemappedVD.push_back({NewValue, V.Count});
10051009
}
1010+
10061011
std::vector<InstrProfValueSiteRecord> &ValueSites =
10071012
getOrCreateValueSitesForKind(ValueKind);
10081013
assert(ValueSites.size() == Site);
1009-
if (N == 0)
1010-
ValueSites.emplace_back();
1011-
else
1012-
ValueSites.emplace_back(VData, VData + N);
1014+
1015+
// Add a new value site with remapped value profiling data.
1016+
ValueSites.emplace_back(std::move(RemappedVD));
10131017
}
10141018

10151019
void TemporalProfTraceTy::createBPFunctionNodes(
@@ -1143,7 +1147,8 @@ void ValueProfRecord::deserializeTo(InstrProfRecord &Record,
11431147
InstrProfValueData *ValueData = getValueProfRecordValueData(this);
11441148
for (uint64_t VSite = 0; VSite < NumValueSites; ++VSite) {
11451149
uint8_t ValueDataCount = this->SiteCountArray[VSite];
1146-
Record.addValueData(Kind, VSite, ValueData, ValueDataCount, SymTab);
1150+
ArrayRef<InstrProfValueData> VDs(ValueData, ValueDataCount);
1151+
Record.addValueData(Kind, VSite, VDs, SymTab);
11471152
ValueData += ValueDataCount;
11481153
}
11491154
}

‎llvm/lib/ProfileData/InstrProfReader.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,8 @@ TextInstrProfReader::readValueProfileData(InstrProfRecord &Record) {
378378
CurrentValues.push_back({Value, TakenCount});
379379
Line++;
380380
}
381-
Record.addValueData(ValueKind, S, CurrentValues.data(), NumValueData,
382-
nullptr);
381+
assert(CurrentValues.size() == NumValueData);
382+
Record.addValueData(ValueKind, S, CurrentValues, nullptr);
383383
}
384384
}
385385
return success();

‎llvm/unittests/ProfileData/InstrProfTest.cpp

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -808,13 +808,13 @@ TEST_P(InstrProfReaderWriterTest, icall_and_vtable_data_read_write) {
808808
Record1.reserveSites(IPVK_IndirectCallTarget, 4);
809809
InstrProfValueData VD0[] = {
810810
{(uint64_t)callee1, 1}, {(uint64_t)callee2, 2}, {(uint64_t)callee3, 3}};
811-
Record1.addValueData(IPVK_IndirectCallTarget, 0, VD0, 3, nullptr);
811+
Record1.addValueData(IPVK_IndirectCallTarget, 0, VD0, nullptr);
812812
// No value profile data at the second site.
813-
Record1.addValueData(IPVK_IndirectCallTarget, 1, nullptr, 0, nullptr);
813+
Record1.addValueData(IPVK_IndirectCallTarget, 1, std::nullopt, nullptr);
814814
InstrProfValueData VD2[] = {{(uint64_t)callee1, 1}, {(uint64_t)callee2, 2}};
815-
Record1.addValueData(IPVK_IndirectCallTarget, 2, VD2, 2, nullptr);
815+
Record1.addValueData(IPVK_IndirectCallTarget, 2, VD2, nullptr);
816816
InstrProfValueData VD3[] = {{(uint64_t)callee7, 1}, {(uint64_t)callee8, 2}};
817-
Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, 2, nullptr);
817+
Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, nullptr);
818818
}
819819

820820
// 2 vtable value sites.
@@ -828,8 +828,8 @@ TEST_P(InstrProfReaderWriterTest, icall_and_vtable_data_read_write) {
828828
{getCalleeAddress(vtable1), 1},
829829
{getCalleeAddress(vtable2), 2},
830830
};
831-
Record1.addValueData(IPVK_VTableTarget, 0, VD0, 3, nullptr);
832-
Record1.addValueData(IPVK_VTableTarget, 1, VD2, 2, nullptr);
831+
Record1.addValueData(IPVK_VTableTarget, 0, VD0, nullptr);
832+
Record1.addValueData(IPVK_VTableTarget, 1, VD2, nullptr);
833833
}
834834

835835
Writer.addRecord(std::move(Record1), getProfWeight(), Err);
@@ -918,7 +918,7 @@ TEST_P(MaybeSparseInstrProfTest, annotate_vp_data) {
918918
Record.reserveSites(IPVK_IndirectCallTarget, 1);
919919
InstrProfValueData VD0[] = {{1000, 1}, {2000, 2}, {3000, 3}, {5000, 5},
920920
{4000, 4}, {6000, 6}};
921-
Record.addValueData(IPVK_IndirectCallTarget, 0, VD0, 6, nullptr);
921+
Record.addValueData(IPVK_IndirectCallTarget, 0, VD0, nullptr);
922922
Writer.addRecord(std::move(Record), Err);
923923
auto Profile = Writer.writeBuffer();
924924
readProfile(std::move(Profile));
@@ -1011,21 +1011,21 @@ TEST_P(MaybeSparseInstrProfTest, icall_and_vtable_data_merge) {
10111011
{uint64_t(callee2), 2},
10121012
{uint64_t(callee3), 3},
10131013
{uint64_t(callee4), 4}};
1014-
Record11.addValueData(IPVK_IndirectCallTarget, 0, VD0, 4, nullptr);
1014+
Record11.addValueData(IPVK_IndirectCallTarget, 0, VD0, nullptr);
10151015

10161016
// No value profile data at the second site.
1017-
Record11.addValueData(IPVK_IndirectCallTarget, 1, nullptr, 0, nullptr);
1017+
Record11.addValueData(IPVK_IndirectCallTarget, 1, std::nullopt, nullptr);
10181018

10191019
InstrProfValueData VD2[] = {
10201020
{uint64_t(callee1), 1}, {uint64_t(callee2), 2}, {uint64_t(callee3), 3}};
1021-
Record11.addValueData(IPVK_IndirectCallTarget, 2, VD2, 3, nullptr);
1021+
Record11.addValueData(IPVK_IndirectCallTarget, 2, VD2, nullptr);
10221022

10231023
InstrProfValueData VD3[] = {{uint64_t(callee7), 1}, {uint64_t(callee8), 2}};
1024-
Record11.addValueData(IPVK_IndirectCallTarget, 3, VD3, 2, nullptr);
1024+
Record11.addValueData(IPVK_IndirectCallTarget, 3, VD3, nullptr);
10251025

10261026
InstrProfValueData VD4[] = {
10271027
{uint64_t(callee1), 1}, {uint64_t(callee2), 2}, {uint64_t(callee3), 3}};
1028-
Record11.addValueData(IPVK_IndirectCallTarget, 4, VD4, 3, nullptr);
1028+
Record11.addValueData(IPVK_IndirectCallTarget, 4, VD4, nullptr);
10291029
}
10301030
// 3 value sites for vtables.
10311031
{
@@ -1034,53 +1034,53 @@ TEST_P(MaybeSparseInstrProfTest, icall_and_vtable_data_merge) {
10341034
{getCalleeAddress(vtable2), 2},
10351035
{getCalleeAddress(vtable3), 3},
10361036
{getCalleeAddress(vtable4), 4}};
1037-
Record11.addValueData(IPVK_VTableTarget, 0, VD0, 4, nullptr);
1037+
Record11.addValueData(IPVK_VTableTarget, 0, VD0, nullptr);
10381038

10391039
InstrProfValueData VD2[] = {{getCalleeAddress(vtable1), 1},
10401040
{getCalleeAddress(vtable2), 2},
10411041
{getCalleeAddress(vtable3), 3}};
1042-
Record11.addValueData(IPVK_VTableTarget, 1, VD2, 3, nullptr);
1042+
Record11.addValueData(IPVK_VTableTarget, 1, VD2, nullptr);
10431043

10441044
InstrProfValueData VD4[] = {{getCalleeAddress(vtable1), 1},
10451045
{getCalleeAddress(vtable2), 2},
10461046
{getCalleeAddress(vtable3), 3}};
1047-
Record11.addValueData(IPVK_VTableTarget, 2, VD4, 3, nullptr);
1047+
Record11.addValueData(IPVK_VTableTarget, 2, VD4, nullptr);
10481048
}
10491049

10501050
// A different record for the same caller.
10511051
Record12.reserveSites(IPVK_IndirectCallTarget, 5);
10521052
InstrProfValueData VD02[] = {{uint64_t(callee2), 5}, {uint64_t(callee3), 3}};
1053-
Record12.addValueData(IPVK_IndirectCallTarget, 0, VD02, 2, nullptr);
1053+
Record12.addValueData(IPVK_IndirectCallTarget, 0, VD02, nullptr);
10541054

10551055
// No value profile data at the second site.
1056-
Record12.addValueData(IPVK_IndirectCallTarget, 1, nullptr, 0, nullptr);
1056+
Record12.addValueData(IPVK_IndirectCallTarget, 1, std::nullopt, nullptr);
10571057

10581058
InstrProfValueData VD22[] = {
10591059
{uint64_t(callee2), 1}, {uint64_t(callee3), 3}, {uint64_t(callee4), 4}};
1060-
Record12.addValueData(IPVK_IndirectCallTarget, 2, VD22, 3, nullptr);
1060+
Record12.addValueData(IPVK_IndirectCallTarget, 2, VD22, nullptr);
10611061

1062-
Record12.addValueData(IPVK_IndirectCallTarget, 3, nullptr, 0, nullptr);
1062+
Record12.addValueData(IPVK_IndirectCallTarget, 3, std::nullopt, nullptr);
10631063

10641064
InstrProfValueData VD42[] = {
10651065
{uint64_t(callee1), 1}, {uint64_t(callee2), 2}, {uint64_t(callee3), 3}};
1066-
Record12.addValueData(IPVK_IndirectCallTarget, 4, VD42, 3, nullptr);
1066+
Record12.addValueData(IPVK_IndirectCallTarget, 4, VD42, nullptr);
10671067

10681068
// 3 value sites for vtables.
10691069
{
10701070
Record12.reserveSites(IPVK_VTableTarget, 3);
10711071
InstrProfValueData VD0[] = {{getCalleeAddress(vtable2), 5},
10721072
{getCalleeAddress(vtable3), 3}};
1073-
Record12.addValueData(IPVK_VTableTarget, 0, VD0, 2, nullptr);
1073+
Record12.addValueData(IPVK_VTableTarget, 0, VD0, nullptr);
10741074

10751075
InstrProfValueData VD2[] = {{getCalleeAddress(vtable2), 1},
10761076
{getCalleeAddress(vtable3), 3},
10771077
{getCalleeAddress(vtable4), 4}};
1078-
Record12.addValueData(IPVK_VTableTarget, 1, VD2, 3, nullptr);
1078+
Record12.addValueData(IPVK_VTableTarget, 1, VD2, nullptr);
10791079

10801080
InstrProfValueData VD4[] = {{getCalleeAddress(vtable1), 1},
10811081
{getCalleeAddress(vtable2), 2},
10821082
{getCalleeAddress(vtable3), 3}};
1083-
Record12.addValueData(IPVK_VTableTarget, 2, VD4, 3, nullptr);
1083+
Record12.addValueData(IPVK_VTableTarget, 2, VD4, nullptr);
10841084
}
10851085

10861086
Writer.addRecord(std::move(Record11), Err);
@@ -1220,7 +1220,7 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_saturation) {
12201220
NamedInstrProfRecord Record4("baz", 0x5678, {3, 4});
12211221
Record4.reserveSites(ValueKind, 1);
12221222
InstrProfValueData VD4[] = {{ProfiledValue, 1}};
1223-
Record4.addValueData(ValueKind, 0, VD4, 1, nullptr);
1223+
Record4.addValueData(ValueKind, 0, VD4, nullptr);
12241224
Result = instrprof_error::success;
12251225
Writer.addRecord(std::move(Record4), Err);
12261226
ASSERT_EQ(Result, instrprof_error::success);
@@ -1229,7 +1229,7 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_saturation) {
12291229
NamedInstrProfRecord Record5("baz", 0x5678, {5, 6});
12301230
Record5.reserveSites(ValueKind, 1);
12311231
InstrProfValueData VD5[] = {{ProfiledValue, MaxValCount}};
1232-
Record5.addValueData(ValueKind, 0, VD5, 1, nullptr);
1232+
Record5.addValueData(ValueKind, 0, VD5, nullptr);
12331233
Result = instrprof_error::success;
12341234
Writer.addRecord(std::move(Record5), Err);
12351235
ASSERT_EQ(Result, instrprof_error::counter_overflow);
@@ -1269,8 +1269,8 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_site_trunc) {
12691269
VD0[I].Count = 2 * I + 1000;
12701270
}
12711271

1272-
Record11.addValueData(ValueKind, 0, VD0, 255, nullptr);
1273-
Record11.addValueData(ValueKind, 1, nullptr, 0, nullptr);
1272+
Record11.addValueData(ValueKind, 0, VD0, nullptr);
1273+
Record11.addValueData(ValueKind, 1, std::nullopt, nullptr);
12741274

12751275
Record12.reserveSites(ValueKind, 2);
12761276
InstrProfValueData VD1[255];
@@ -1279,8 +1279,8 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_site_trunc) {
12791279
VD1[I].Count = 2 * I + 1001;
12801280
}
12811281

1282-
Record12.addValueData(ValueKind, 0, VD1, 255, nullptr);
1283-
Record12.addValueData(ValueKind, 1, nullptr, 0, nullptr);
1282+
Record12.addValueData(ValueKind, 0, VD1, nullptr);
1283+
Record12.addValueData(ValueKind, 1, std::nullopt, nullptr);
12841284

12851285
Writer.addRecord(std::move(Record11), Err);
12861286
// Merge profile data.
@@ -1317,23 +1317,23 @@ static void addValueProfData(InstrProfRecord &Record) {
13171317
{uint64_t(callee3), 500},
13181318
{uint64_t(callee4), 300},
13191319
{uint64_t(callee5), 100}};
1320-
Record.addValueData(IPVK_IndirectCallTarget, 0, VD0, 5, nullptr);
1320+
Record.addValueData(IPVK_IndirectCallTarget, 0, VD0, nullptr);
13211321
InstrProfValueData VD1[] = {{uint64_t(callee5), 800},
13221322
{uint64_t(callee3), 1000},
13231323
{uint64_t(callee2), 2500},
13241324
{uint64_t(callee1), 1300}};
1325-
Record.addValueData(IPVK_IndirectCallTarget, 1, VD1, 4, nullptr);
1325+
Record.addValueData(IPVK_IndirectCallTarget, 1, VD1, nullptr);
13261326
InstrProfValueData VD2[] = {{uint64_t(callee6), 800},
13271327
{uint64_t(callee3), 1000},
13281328
{uint64_t(callee4), 5500}};
1329-
Record.addValueData(IPVK_IndirectCallTarget, 2, VD2, 3, nullptr);
1329+
Record.addValueData(IPVK_IndirectCallTarget, 2, VD2, nullptr);
13301330
InstrProfValueData VD3[] = {{uint64_t(callee2), 1800},
13311331
{uint64_t(callee3), 2000}};
1332-
Record.addValueData(IPVK_IndirectCallTarget, 3, VD3, 2, nullptr);
1333-
Record.addValueData(IPVK_IndirectCallTarget, 4, nullptr, 0, nullptr);
1332+
Record.addValueData(IPVK_IndirectCallTarget, 3, VD3, nullptr);
1333+
Record.addValueData(IPVK_IndirectCallTarget, 4, std::nullopt, nullptr);
13341334
InstrProfValueData VD5[] = {{uint64_t(callee7), 1234},
13351335
{uint64_t(callee8), 5678}};
1336-
Record.addValueData(IPVK_IndirectCallTarget, 5, VD5, 2, nullptr);
1336+
Record.addValueData(IPVK_IndirectCallTarget, 5, VD5, nullptr);
13371337
}
13381338

13391339
// Add test data for vtables
@@ -1355,10 +1355,10 @@ static void addValueProfData(InstrProfRecord &Record) {
13551355
};
13561356
InstrProfValueData VD3[] = {{getCalleeAddress(vtable2), 1800},
13571357
{getCalleeAddress(vtable3), 2000}};
1358-
Record.addValueData(IPVK_VTableTarget, 0, VD0, 5, nullptr);
1359-
Record.addValueData(IPVK_VTableTarget, 1, VD1, 4, nullptr);
1360-
Record.addValueData(IPVK_VTableTarget, 2, VD2, 3, nullptr);
1361-
Record.addValueData(IPVK_VTableTarget, 3, VD3, 2, nullptr);
1358+
Record.addValueData(IPVK_VTableTarget, 0, VD0, nullptr);
1359+
Record.addValueData(IPVK_VTableTarget, 1, VD1, nullptr);
1360+
Record.addValueData(IPVK_VTableTarget, 2, VD2, nullptr);
1361+
Record.addValueData(IPVK_VTableTarget, 3, VD3, nullptr);
13621362
}
13631363
}
13641364

0 commit comments

Comments
 (0)