[clang] f18cfb9 - [InstrProf] Factor out getRecord() and use NamedInstrProfRecord (#145417)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 24 09:52:51 PDT 2025
Author: Ellis Hoag
Date: 2025-06-24T09:52:47-07:00
New Revision: f18cfb9108fda51c7c8233c32b4e2193a0a13766
URL: https://github.com/llvm/llvm-project/commit/f18cfb9108fda51c7c8233c32b4e2193a0a13766
DIFF: https://github.com/llvm/llvm-project/commit/f18cfb9108fda51c7c8233c32b4e2193a0a13766.diff
LOG: [InstrProf] Factor out getRecord() and use NamedInstrProfRecord (#145417)
Factor out code in `populateCounters()` and `populateCoverage()` used to
grab the record into `PGOUseFunc::getRecord()` to reduce code
duplication. And return `NamedInstrProfRecord` in `getInstrProfRecord()`
to avoid an unnecessary cast. No functional change is intented.
Added:
Modified:
clang/lib/CodeGen/CodeGenPGO.cpp
llvm/include/llvm/ProfileData/InstrProfReader.h
llvm/lib/ProfileData/InstrProfReader.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index a80bebbb4cf41..98b30e084b18b 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -1423,8 +1423,7 @@ void CodeGenPGO::loadRegionCounts(llvm::IndexedInstrProfReader *PGOReader,
bool IsInMainFile) {
CGM.getPGOStats().addVisited(IsInMainFile);
RegionCounts.clear();
- llvm::Expected<llvm::InstrProfRecord> RecordExpected =
- PGOReader->getInstrProfRecord(FuncName, FunctionHash);
+ auto RecordExpected = PGOReader->getInstrProfRecord(FuncName, FunctionHash);
if (auto E = RecordExpected.takeError()) {
auto IPE = std::get<0>(llvm::InstrProfError::take(std::move(E)));
if (IPE == llvm::instrprof_error::unknown_function)
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
index 3b12505d3326c..deb5cd17d8fd9 100644
--- a/llvm/include/llvm/ProfileData/InstrProfReader.h
+++ b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -824,7 +824,7 @@ class LLVM_ABI IndexedInstrProfReader : public InstrProfReader {
/// functions, MismatchedFuncSum returns the maximum. If \c FuncName is not
/// found, try to lookup \c DeprecatedFuncName to handle profiles built by
/// older compilers.
- Expected<InstrProfRecord>
+ Expected<NamedInstrProfRecord>
getInstrProfRecord(StringRef FuncName, uint64_t FuncHash,
StringRef DeprecatedFuncName = "",
uint64_t *MismatchedFuncSum = nullptr);
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index ab109cd5b13a7..5c7b9e0544030 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -1369,7 +1369,7 @@ InstrProfSymtab &IndexedInstrProfReader::getSymtab() {
return *Symtab;
}
-Expected<InstrProfRecord> IndexedInstrProfReader::getInstrProfRecord(
+Expected<NamedInstrProfRecord> IndexedInstrProfReader::getInstrProfRecord(
StringRef FuncName, uint64_t FuncHash, StringRef DeprecatedFuncName,
uint64_t *MismatchedFuncSum) {
ArrayRef<NamedInstrProfRecord> Data;
@@ -1572,7 +1572,7 @@ memprof::AllMemProfData IndexedMemProfReader::getAllMemProfData() const {
Error IndexedInstrProfReader::getFunctionCounts(StringRef FuncName,
uint64_t FuncHash,
std::vector<uint64_t> &Counts) {
- Expected<InstrProfRecord> Record = getInstrProfRecord(FuncName, FuncHash);
+ auto Record = getInstrProfRecord(FuncName, FuncHash);
if (Error E = Record.takeError())
return error(std::move(E));
@@ -1583,7 +1583,7 @@ Error IndexedInstrProfReader::getFunctionCounts(StringRef FuncName,
Error IndexedInstrProfReader::getFunctionBitmap(StringRef FuncName,
uint64_t FuncHash,
BitVector &Bitmap) {
- Expected<InstrProfRecord> Record = getInstrProfRecord(FuncName, FuncHash);
+ auto Record = getInstrProfRecord(FuncName, FuncHash);
if (Error E = Record.takeError())
return error(std::move(E));
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 9c92a7feb6e61..6f06a260e238c 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1176,15 +1176,20 @@ class PGOUseFunc {
void handleInstrProfError(Error Err, uint64_t MismatchedFuncSum);
+ /// Get the profile record, assign it to \p ProfileRecord, handle errors if
+ /// necessary, and assign \p ProgramMaxCount. \returns true if there are no
+ /// errors.
+ bool getRecord(IndexedInstrProfReader *PGOReader);
+
// Read counts for the instrumented BB from profile.
- bool readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
+ bool readCounters(bool &AllZeros,
InstrProfRecord::CountPseudoKind &PseudoKind);
// Populate the counts for all BBs.
void populateCounters();
// Set block coverage based on profile coverage values.
- void populateCoverage(IndexedInstrProfReader *PGOReader);
+ void populateCoverage();
// Set the branch weights based on the count values.
void setBranchWeights();
@@ -1208,7 +1213,7 @@ class PGOUseFunc {
uint64_t getFuncHash() const { return FuncInfo.FunctionHash; }
// Return the profile record for this function;
- InstrProfRecord &getProfileRecord() { return ProfileRecord; }
+ NamedInstrProfRecord &getProfileRecord() { return ProfileRecord; }
// Return the auxiliary BB information.
PGOUseBBInfo &getBBInfo(const BasicBlock *BB) const {
@@ -1246,7 +1251,7 @@ class PGOUseFunc {
uint32_t ProfileCountSize = 0;
// ProfileRecord for this function.
- InstrProfRecord ProfileRecord;
+ NamedInstrProfRecord ProfileRecord;
// Function hotness info derived from profile.
FuncFreqAttr FreqAttr;
@@ -1441,14 +1446,9 @@ void PGOUseFunc::handleInstrProfError(Error Err, uint64_t MismatchedFuncSum) {
});
}
-// Read the profile from ProfileFileName and assign the value to the
-// instrumented BB and the edges. This function also updates ProgramMaxCount.
-// Return true if the profile are successfully read, and false on errors.
-bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
- InstrProfRecord::CountPseudoKind &PseudoKind) {
- auto &Ctx = M->getContext();
+bool PGOUseFunc::getRecord(IndexedInstrProfReader *PGOReader) {
uint64_t MismatchedFuncSum = 0;
- Expected<InstrProfRecord> Result = PGOReader->getInstrProfRecord(
+ auto Result = PGOReader->getInstrProfRecord(
FuncInfo.FuncName, FuncInfo.FunctionHash, FuncInfo.DeprecatedFuncName,
&MismatchedFuncSum);
if (Error E = Result.takeError()) {
@@ -1456,6 +1456,16 @@ bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
return false;
}
ProfileRecord = std::move(Result.get());
+ ProgramMaxCount = PGOReader->getMaximumFunctionCount(IsCS);
+ return true;
+}
+
+// Read the profile from ProfileFileName and assign the value to the
+// instrumented BB and the edges. Return true if the profile are successfully
+// read, and false on errors.
+bool PGOUseFunc::readCounters(bool &AllZeros,
+ InstrProfRecord::CountPseudoKind &PseudoKind) {
+ auto &Ctx = M->getContext();
PseudoKind = ProfileRecord.getCountPseudoKind();
if (PseudoKind != InstrProfRecord::NotPseudo) {
return true;
@@ -1488,22 +1498,13 @@ bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
DS_Warning));
return false;
}
- ProgramMaxCount = PGOReader->getMaximumFunctionCount(IsCS);
return true;
}
-void PGOUseFunc::populateCoverage(IndexedInstrProfReader *PGOReader) {
- uint64_t MismatchedFuncSum = 0;
- Expected<InstrProfRecord> Result = PGOReader->getInstrProfRecord(
- FuncInfo.FuncName, FuncInfo.FunctionHash, FuncInfo.DeprecatedFuncName,
- &MismatchedFuncSum);
- if (auto Err = Result.takeError()) {
- handleInstrProfError(std::move(Err), MismatchedFuncSum);
- return;
- }
+void PGOUseFunc::populateCoverage() {
IsCS ? NumOfCSPGOFunc++ : NumOfPGOFunc++;
- std::vector<uint64_t> &CountsFromProfile = Result.get().Counts;
+ ArrayRef<uint64_t> CountsFromProfile = ProfileRecord.Counts;
DenseMap<const BasicBlock *, bool> Coverage;
unsigned Index = 0;
for (auto &BB : F)
@@ -2243,8 +2244,10 @@ static bool annotateAllFunctions(
PGOUseFunc Func(F, &M, TLI, ComdatMembers, BPI, BFI, LI, PSI, IsCS,
InstrumentFuncEntry, InstrumentLoopEntries,
HasSingleByteCoverage);
+ if (!Func.getRecord(PGOReader.get()))
+ continue;
if (HasSingleByteCoverage) {
- Func.populateCoverage(PGOReader.get());
+ Func.populateCoverage();
continue;
}
// When PseudoKind is set to a vaule other than InstrProfRecord::NotPseudo,
@@ -2253,7 +2256,7 @@ static bool annotateAllFunctions(
// attribute and drop all the profile counters.
InstrProfRecord::CountPseudoKind PseudoKind = InstrProfRecord::NotPseudo;
bool AllZeros = false;
- if (!Func.readCounters(PGOReader.get(), AllZeros, PseudoKind))
+ if (!Func.readCounters(AllZeros, PseudoKind))
continue;
if (AllZeros) {
F.setEntryCount(ProfileCount(0, Function::PCT_Real));
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index dcdacb903791d..6467695355be8 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -135,7 +135,7 @@ TEST_P(MaybeSparseInstrProfTest, get_instr_prof_record) {
auto Profile = Writer.writeBuffer();
readProfile(std::move(Profile));
- Expected<InstrProfRecord> R = Reader->getInstrProfRecord("foo", 0x1234);
+ auto R = Reader->getInstrProfRecord("foo", 0x1234);
EXPECT_THAT_ERROR(R.takeError(), Succeeded());
ASSERT_EQ(2U, R->Counts.size());
ASSERT_EQ(1U, R->Counts[0]);
@@ -251,7 +251,7 @@ TEST_F(InstrProfTest, test_writer_merge) {
auto Profile = Writer.writeBuffer();
readProfile(std::move(Profile));
- Expected<InstrProfRecord> R = Reader->getInstrProfRecord("func1", 0x1234);
+ auto R = Reader->getInstrProfRecord("func1", 0x1234);
EXPECT_THAT_ERROR(R.takeError(), Succeeded());
ASSERT_EQ(1U, R->Counts.size());
ASSERT_EQ(42U, R->Counts[0]);
@@ -600,7 +600,7 @@ TEST_F(InstrProfTest, test_memprof_merge) {
auto Profile = Writer.writeBuffer();
readProfile(std::move(Profile));
- Expected<InstrProfRecord> R = Reader->getInstrProfRecord("func1", 0x1234);
+ auto R = Reader->getInstrProfRecord("func1", 0x1234);
EXPECT_THAT_ERROR(R.takeError(), Succeeded());
ASSERT_EQ(1U, R->Counts.size());
ASSERT_EQ(42U, R->Counts[0]);
@@ -800,7 +800,7 @@ TEST_P(InstrProfReaderWriterTest, icall_and_vtable_data_read_write) {
// Set reader value prof data endianness.
Reader->setValueProfDataEndianness(getEndianness());
- Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
+ auto R = Reader->getInstrProfRecord("caller", 0x1234);
ASSERT_THAT_ERROR(R.takeError(), Succeeded());
// Test the number of instrumented indirect call sites and the number of
@@ -874,7 +874,7 @@ TEST_P(MaybeSparseInstrProfTest, annotate_vp_data) {
Writer.addRecord(std::move(Record), Err);
auto Profile = Writer.writeBuffer();
readProfile(std::move(Profile));
- Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
+ auto R = Reader->getInstrProfRecord("caller", 0x1234);
EXPECT_THAT_ERROR(R.takeError(), Succeeded());
LLVMContext Ctx;
@@ -1051,7 +1051,7 @@ TEST_P(MaybeSparseInstrProfTest, icall_and_vtable_data_merge) {
// Test the number of instrumented value sites and the number of profiled
// values for each site.
- Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
+ auto R = Reader->getInstrProfRecord("caller", 0x1234);
EXPECT_THAT_ERROR(R.takeError(), Succeeded());
// For indirect calls.
ASSERT_EQ(5U, R->getNumValueSites(IPVK_IndirectCallTarget));
@@ -1190,13 +1190,11 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_saturation) {
readProfile(std::move(Profile));
// Verify saturation of counts.
- Expected<InstrProfRecord> ReadRecord1 =
- Reader->getInstrProfRecord("foo", 0x1234);
+ auto ReadRecord1 = Reader->getInstrProfRecord("foo", 0x1234);
ASSERT_THAT_ERROR(ReadRecord1.takeError(), Succeeded());
EXPECT_EQ(MaxEdgeCount, ReadRecord1->Counts[0]);
- Expected<InstrProfRecord> ReadRecord2 =
- Reader->getInstrProfRecord("baz", 0x5678);
+ auto ReadRecord2 = Reader->getInstrProfRecord("baz", 0x5678);
ASSERT_TRUE(bool(ReadRecord2));
ASSERT_EQ(1U, ReadRecord2->getNumValueSites(ValueKind));
auto VD = ReadRecord2->getValueArrayForSite(ValueKind, 0);
@@ -1241,7 +1239,7 @@ TEST_P(ValueProfileMergeEdgeCaseTest, value_profile_data_merge_site_trunc) {
auto Profile = Writer.writeBuffer();
readProfile(std::move(Profile));
- Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
+ auto R = Reader->getInstrProfRecord("caller", 0x1234);
ASSERT_THAT_ERROR(R.takeError(), Succeeded());
ASSERT_EQ(2U, R->getNumValueSites(ValueKind));
auto VD = R->getValueArrayForSite(ValueKind, 0);
More information about the cfe-commits
mailing list