Skip to content

Commit a32c2c3

Browse files
committed
[NFC] Use Optional<ProfileCount> to model invalid counts
ProfileCount could model invalid values, but a user had no indication that the getCount method could return bogus data. Optional<ProfileCount> addresses that, because the user must dereference the optional. In addition, the patch removes concept duplication. Differential Revision: https://reviews.llvm.org/D113839
1 parent eec9ca6 commit a32c2c3

File tree

10 files changed

+39
-50
lines changed

10 files changed

+39
-50
lines changed

llvm/include/llvm/IR/Function.h

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -247,33 +247,22 @@ class LLVM_EXTERNAL_VISIBILITY Function : public GlobalObject,
247247
setValueSubclassData((getSubclassDataFromValue() & 0xc00f) | (ID << 4));
248248
}
249249

250-
enum ProfileCountType { PCT_Invalid, PCT_Real, PCT_Synthetic };
250+
enum ProfileCountType { PCT_Real, PCT_Synthetic };
251251

252252
/// Class to represent profile counts.
253253
///
254254
/// This class represents both real and synthetic profile counts.
255255
class ProfileCount {
256256
private:
257-
uint64_t Count;
258-
ProfileCountType PCT;
259-
static ProfileCount Invalid;
257+
uint64_t Count = 0;
258+
ProfileCountType PCT = PCT_Real;
260259

261260
public:
262-
ProfileCount() : Count(-1), PCT(PCT_Invalid) {}
263261
ProfileCount(uint64_t Count, ProfileCountType PCT)
264262
: Count(Count), PCT(PCT) {}
265-
bool hasValue() const { return PCT != PCT_Invalid; }
266263
uint64_t getCount() const { return Count; }
267264
ProfileCountType getType() const { return PCT; }
268265
bool isSynthetic() const { return PCT == PCT_Synthetic; }
269-
explicit operator bool() { return hasValue(); }
270-
bool operator!() const { return !hasValue(); }
271-
// Update the count retaining the same profile count type.
272-
ProfileCount &setCount(uint64_t C) {
273-
Count = C;
274-
return *this;
275-
}
276-
static ProfileCount getInvalid() { return ProfileCount(-1, PCT_Invalid); }
277266
};
278267

279268
/// Set the entry count for this function.
@@ -293,7 +282,7 @@ class LLVM_EXTERNAL_VISIBILITY Function : public GlobalObject,
293282
///
294283
/// Entry count is the number of times the function was executed.
295284
/// When AllowSynthetic is false, only pgo_data will be returned.
296-
ProfileCount getEntryCount(bool AllowSynthetic = false) const;
285+
Optional<ProfileCount> getEntryCount(bool AllowSynthetic = false) const;
297286

298287
/// Return true if the function is annotated with profile data.
299288
///

llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ BlockFrequencyInfoImplBase::getProfileCountFromFreq(const Function &F,
602602
if (!EntryCount)
603603
return None;
604604
// Use 128 bit APInt to do the arithmetic to avoid overflow.
605-
APInt BlockCount(128, EntryCount.getCount());
605+
APInt BlockCount(128, EntryCount->getCount());
606606
APInt BlockFreq(128, Freq);
607607
APInt EntryFreq(128, getEntryFreq());
608608
BlockCount *= BlockFreq;

llvm/lib/Analysis/InlineCost.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ class InlineCostCallAnalyzer final : public CallAnalyzer {
771771

772772
// Make sure we have a nonzero entry count.
773773
auto EntryCount = F.getEntryCount();
774-
if (!EntryCount || !EntryCount.getCount())
774+
if (!EntryCount || !EntryCount->getCount())
775775
return false;
776776

777777
BlockFrequencyInfo *CalleeBFI = &(GetBFI(F));
@@ -837,8 +837,8 @@ class InlineCostCallAnalyzer final : public CallAnalyzer {
837837

838838
// Compute the cycle savings per call.
839839
auto EntryProfileCount = F.getEntryCount();
840-
assert(EntryProfileCount.hasValue() && EntryProfileCount.getCount());
841-
auto EntryCount = EntryProfileCount.getCount();
840+
assert(EntryProfileCount.hasValue() && EntryProfileCount->getCount());
841+
auto EntryCount = EntryProfileCount->getCount();
842842
CycleSavings += EntryCount / 2;
843843
CycleSavings = CycleSavings.udiv(EntryCount);
844844

llvm/lib/Analysis/ProfileSummaryInfo.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ bool ProfileSummaryInfo::isFunctionEntryHot(const Function *F) const {
103103
// FIXME: The heuristic used below for determining hotness is based on
104104
// preliminary SPEC tuning for inliner. This will eventually be a
105105
// convenience method that calls isHotCount.
106-
return FunctionCount && isHotCount(FunctionCount.getCount());
106+
return FunctionCount && isHotCount(FunctionCount->getCount());
107107
}
108108

109109
/// Returns true if the function contains hot code. This can include a hot
@@ -116,7 +116,7 @@ bool ProfileSummaryInfo::isFunctionHotInCallGraph(
116116
if (!F || !hasProfileSummary())
117117
return false;
118118
if (auto FunctionCount = F->getEntryCount())
119-
if (isHotCount(FunctionCount.getCount()))
119+
if (isHotCount(FunctionCount->getCount()))
120120
return true;
121121

122122
if (hasSampleProfile()) {
@@ -145,7 +145,7 @@ bool ProfileSummaryInfo::isFunctionColdInCallGraph(
145145
if (!F || !hasProfileSummary())
146146
return false;
147147
if (auto FunctionCount = F->getEntryCount())
148-
if (!isColdCount(FunctionCount.getCount()))
148+
if (!isColdCount(FunctionCount->getCount()))
149149
return false;
150150

151151
if (hasSampleProfile()) {
@@ -176,10 +176,10 @@ bool ProfileSummaryInfo::isFunctionHotOrColdInCallGraphNthPercentile(
176176
return false;
177177
if (auto FunctionCount = F->getEntryCount()) {
178178
if (isHot &&
179-
isHotCountNthPercentile(PercentileCutoff, FunctionCount.getCount()))
179+
isHotCountNthPercentile(PercentileCutoff, FunctionCount->getCount()))
180180
return true;
181181
if (!isHot &&
182-
!isColdCountNthPercentile(PercentileCutoff, FunctionCount.getCount()))
182+
!isColdCountNthPercentile(PercentileCutoff, FunctionCount->getCount()))
183183
return false;
184184
}
185185
if (hasSampleProfile()) {
@@ -230,7 +230,7 @@ bool ProfileSummaryInfo::isFunctionEntryCold(const Function *F) const {
230230
// FIXME: The heuristic used below for determining coldness is based on
231231
// preliminary SPEC tuning for inliner. This will eventually be a
232232
// convenience method that calls isHotCount.
233-
return FunctionCount && isColdCount(FunctionCount.getCount());
233+
return FunctionCount && isColdCount(FunctionCount->getCount());
234234
}
235235

236236
/// Compute the hot and cold thresholds.

llvm/lib/CodeGen/MachineSizeOpts.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ bool isFunctionColdInCallGraph(
8282
ProfileSummaryInfo *PSI,
8383
const MachineBlockFrequencyInfo &MBFI) {
8484
if (auto FunctionCount = MF->getFunction().getEntryCount())
85-
if (!PSI->isColdCount(FunctionCount.getCount()))
85+
if (!PSI->isColdCount(FunctionCount->getCount()))
8686
return false;
8787
for (const auto &MBB : *MF)
8888
if (!isColdBlock(&MBB, PSI, &MBFI))
@@ -99,7 +99,7 @@ bool isFunctionHotInCallGraphNthPercentile(
9999
const MachineBlockFrequencyInfo &MBFI) {
100100
if (auto FunctionCount = MF->getFunction().getEntryCount())
101101
if (PSI->isHotCountNthPercentile(PercentileCutoff,
102-
FunctionCount.getCount()))
102+
FunctionCount->getCount()))
103103
return true;
104104
for (const auto &MBB : *MF)
105105
if (isHotBlockNthPercentile(PercentileCutoff, &MBB, PSI, &MBFI))
@@ -112,7 +112,7 @@ bool isFunctionColdInCallGraphNthPercentile(
112112
const MachineBlockFrequencyInfo &MBFI) {
113113
if (auto FunctionCount = MF->getFunction().getEntryCount())
114114
if (!PSI->isColdCountNthPercentile(PercentileCutoff,
115-
FunctionCount.getCount()))
115+
FunctionCount->getCount()))
116116
return false;
117117
for (const auto &MBB : *MF)
118118
if (!isColdBlockNthPercentile(PercentileCutoff, &MBB, PSI, &MBFI))

llvm/lib/IR/Function.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1892,10 +1892,9 @@ void Function::setValueSubclassDataBit(unsigned Bit, bool On) {
18921892

18931893
void Function::setEntryCount(ProfileCount Count,
18941894
const DenseSet<GlobalValue::GUID> *S) {
1895-
assert(Count.hasValue());
18961895
#if !defined(NDEBUG)
18971896
auto PrevCount = getEntryCount();
1898-
assert(!PrevCount.hasValue() || PrevCount.getType() == Count.getType());
1897+
assert(!PrevCount.hasValue() || PrevCount->getType() == Count.getType());
18991898
#endif
19001899

19011900
auto ImportGUIDs = getImportGUIDs();
@@ -1913,7 +1912,7 @@ void Function::setEntryCount(uint64_t Count, Function::ProfileCountType Type,
19131912
setEntryCount(ProfileCount(Count, Type), Imports);
19141913
}
19151914

1916-
ProfileCount Function::getEntryCount(bool AllowSynthetic) const {
1915+
Optional<ProfileCount> Function::getEntryCount(bool AllowSynthetic) const {
19171916
MDNode *MD = getMetadata(LLVMContext::MD_prof);
19181917
if (MD && MD->getOperand(0))
19191918
if (MDString *MDS = dyn_cast<MDString>(MD->getOperand(0))) {
@@ -1923,7 +1922,7 @@ ProfileCount Function::getEntryCount(bool AllowSynthetic) const {
19231922
// A value of -1 is used for SamplePGO when there were no samples.
19241923
// Treat this the same as unknown.
19251924
if (Count == (uint64_t)-1)
1926-
return ProfileCount::getInvalid();
1925+
return None;
19271926
return ProfileCount(Count, PCT_Real);
19281927
} else if (AllowSynthetic &&
19291928
MDS->getString().equals("synthetic_function_entry_count")) {
@@ -1932,7 +1931,7 @@ ProfileCount Function::getEntryCount(bool AllowSynthetic) const {
19321931
return ProfileCount(Count, PCT_Synthetic);
19331932
}
19341933
}
1935-
return ProfileCount::getInvalid();
1934+
return None;
19361935
}
19371936

19381937
DenseSet<GlobalValue::GUID> Function::getImportGUIDs() const {

llvm/lib/Transforms/IPO/PartialInlining.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,7 +1411,7 @@ bool PartialInlinerImpl::tryPartialInline(FunctionCloner &Cloner) {
14111411
computeCallsiteToProfCountMap(Cloner.ClonedFunc, CallSiteToProfCountMap);
14121412

14131413
uint64_t CalleeEntryCountV =
1414-
(CalleeEntryCount ? CalleeEntryCount.getCount() : 0);
1414+
(CalleeEntryCount ? CalleeEntryCount->getCount() : 0);
14151415

14161416
bool AnyInline = false;
14171417
for (User *User : Users) {
@@ -1459,8 +1459,8 @@ bool PartialInlinerImpl::tryPartialInline(FunctionCloner &Cloner) {
14591459
if (AnyInline) {
14601460
Cloner.IsFunctionInlined = true;
14611461
if (CalleeEntryCount)
1462-
Cloner.OrigFunc->setEntryCount(
1463-
CalleeEntryCount.setCount(CalleeEntryCountV));
1462+
Cloner.OrigFunc->setEntryCount(Function::ProfileCount(
1463+
CalleeEntryCountV, CalleeEntryCount->getType()));
14641464
OptimizationRemarkEmitter OrigFuncORE(Cloner.OrigFunc);
14651465
OrigFuncORE.emit([&]() {
14661466
return OptimizationRemark(DEBUG_TYPE, "PartiallyInlined", Cloner.OrigFunc)

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1686,7 +1686,7 @@ static void fixFuncEntryCount(PGOUseFunc &Func, LoopInfo &LI,
16861686
BlockFrequencyInfo NBFI(F, NBPI, LI);
16871687
#ifndef NDEBUG
16881688
auto BFIEntryCount = F.getEntryCount();
1689-
assert(BFIEntryCount.hasValue() && (BFIEntryCount.getCount() > 0) &&
1689+
assert(BFIEntryCount.hasValue() && (BFIEntryCount->getCount() > 0) &&
16901690
"Invalid BFI Entrycount");
16911691
#endif
16921692
auto SumCount = APFloat::getZero(APFloat::IEEEdouble());

llvm/lib/Transforms/Utils/InlineFunction.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,8 +1600,7 @@ static void updateCallProfile(Function *Callee, const ValueToValueMapTy &VMap,
16001600
const ProfileCount &CalleeEntryCount,
16011601
const CallBase &TheCall, ProfileSummaryInfo *PSI,
16021602
BlockFrequencyInfo *CallerBFI) {
1603-
if (!CalleeEntryCount.hasValue() || CalleeEntryCount.isSynthetic() ||
1604-
CalleeEntryCount.getCount() < 1)
1603+
if (CalleeEntryCount.isSynthetic() || CalleeEntryCount.getCount() < 1)
16051604
return;
16061605
auto CallSiteCount = PSI ? PSI->getProfileCount(TheCall, CallerBFI) : None;
16071606
int64_t CallCount =
@@ -1616,7 +1615,7 @@ void llvm::updateProfileCallee(
16161615
if (!CalleeCount.hasValue())
16171616
return;
16181617

1619-
const uint64_t PriorEntryCount = CalleeCount.getCount();
1618+
const uint64_t PriorEntryCount = CalleeCount->getCount();
16201619

16211620
// Since CallSiteCount is an estimate, it could exceed the original callee
16221621
// count and has to be set to 0 so guard against underflow.
@@ -1969,8 +1968,9 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
19691968
updateCallerBFI(OrigBB, VMap, IFI.CallerBFI, IFI.CalleeBFI,
19701969
CalledFunc->front());
19711970

1972-
updateCallProfile(CalledFunc, VMap, CalledFunc->getEntryCount(), CB,
1973-
IFI.PSI, IFI.CallerBFI);
1971+
if (auto Profile = CalledFunc->getEntryCount())
1972+
updateCallProfile(CalledFunc, VMap, *Profile, CB, IFI.PSI,
1973+
IFI.CallerBFI);
19741974
}
19751975

19761976
// Inject byval arguments initialization.

llvm/unittests/IR/MetadataTest.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3424,23 +3424,24 @@ TEST_F(FunctionAttachmentTest, Verifier) {
34243424
EXPECT_FALSE(verifyFunction(*F));
34253425
}
34263426

3427-
TEST_F(FunctionAttachmentTest, EntryCount) {
3427+
TEST_F(FunctionAttachmentTest, RealEntryCount) {
34283428
Function *F = getFunction("foo");
34293429
EXPECT_FALSE(F->getEntryCount().hasValue());
34303430
F->setEntryCount(12304, Function::PCT_Real);
34313431
auto Count = F->getEntryCount();
34323432
EXPECT_TRUE(Count.hasValue());
3433-
EXPECT_EQ(12304u, Count.getCount());
3434-
EXPECT_EQ(Function::PCT_Real, Count.getType());
3433+
EXPECT_EQ(12304u, Count->getCount());
3434+
EXPECT_EQ(Function::PCT_Real, Count->getType());
3435+
}
34353436

3436-
// Repeat the same for synthetic counts.
3437-
F = getFunction("bar");
3437+
TEST_F(FunctionAttachmentTest, SyntheticEntryCount) {
3438+
Function *F = getFunction("bar");
34383439
EXPECT_FALSE(F->getEntryCount().hasValue());
34393440
F->setEntryCount(123, Function::PCT_Synthetic);
3440-
Count = F->getEntryCount(true /*allow synthetic*/);
3441+
auto Count = F->getEntryCount(true /*allow synthetic*/);
34413442
EXPECT_TRUE(Count.hasValue());
3442-
EXPECT_EQ(123u, Count.getCount());
3443-
EXPECT_EQ(Function::PCT_Synthetic, Count.getType());
3443+
EXPECT_EQ(123u, Count->getCount());
3444+
EXPECT_EQ(Function::PCT_Synthetic, Count->getType());
34443445
}
34453446

34463447
TEST_F(FunctionAttachmentTest, SubprogramAttachment) {

0 commit comments

Comments
 (0)