Skip to content

Commit 6ff8847

Browse files
committed
Detect the order and add a summary flag to the profile
1 parent ccfee2d commit 6ff8847

File tree

7 files changed

+88
-3
lines changed

7 files changed

+88
-3
lines changed

llvm/include/llvm/ProfileData/SampleProf.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,10 @@ enum class SecProfSummaryFlags : uint32_t {
201201
/// SecFlagIsPreInlined means this profile contains ShouldBeInlined
202202
/// contexts thus this is CS preinliner computed.
203203
SecFlagIsPreInlined = (1 << 4),
204+
/// SecFlagIsMixedProbeOrder means in a pseude-probe based profile, the
205+
/// callsite and BB probe IDs are mixed and sorted in lexcial order instead of
206+
/// the order that callsite probe IDs are always after the BB probe IDs.
207+
SecFlagIsMixedProbeOrder = (1 << 5),
204208
};
205209

206210
enum class SecFuncMetadataFlags : uint32_t {
@@ -466,7 +470,7 @@ struct SampleContextFrame {
466470
LineLocation Location;
467471

468472
SampleContextFrame() : Location(0, 0) {}
469-
473+
470474
SampleContextFrame(FunctionId Func, LineLocation Location)
471475
: Func(Func), Location(Location) {}
472476

@@ -527,7 +531,7 @@ class SampleContext {
527531
: Func(Name), State(UnknownContext), Attributes(ContextNone) {
528532
assert(!Name.empty() && "Name is empty");
529533
}
530-
534+
531535
SampleContext(FunctionId Func)
532536
: Func(Func), State(UnknownContext), Attributes(ContextNone) {}
533537

@@ -1178,6 +1182,8 @@ class FunctionSamples {
11781182

11791183
static bool ProfileIsProbeBased;
11801184

1185+
static bool ProfileIsMixedProbeOrder;
1186+
11811187
static bool ProfileIsCS;
11821188

11831189
static bool ProfileIsPreInlined;

llvm/include/llvm/ProfileData/SampleProfReader.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,9 @@ class SampleProfileReader {
525525
/// \brief Whether samples are collected based on pseudo probes.
526526
bool ProfileIsProbeBased = false;
527527

528+
/// Whether profiles are in mixed BB and callsite probe order.
529+
bool ProfileIsMixedProbeOrder = false;
530+
528531
/// Whether function profiles are context-sensitive flat profiles.
529532
bool ProfileIsCS = false;
530533

llvm/lib/ProfileData/SampleProf.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ static cl::opt<bool> GenerateMergedBaseProfiles(
4141
namespace llvm {
4242
namespace sampleprof {
4343
bool FunctionSamples::ProfileIsProbeBased = false;
44+
bool FunctionSamples::ProfileIsMixedProbeOrder = false;
4445
bool FunctionSamples::ProfileIsCS = false;
4546
bool FunctionSamples::ProfileIsPreInlined = false;
4647
bool FunctionSamples::UseMD5 = false;

llvm/lib/ProfileData/SampleProfReader.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,9 @@ std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
704704
FunctionSamples::ProfileIsPreInlined = ProfileIsPreInlined = true;
705705
if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagFSDiscriminator))
706706
FunctionSamples::ProfileIsFS = ProfileIsFS = true;
707+
if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagIsMixedProbeOrder))
708+
FunctionSamples::ProfileIsMixedProbeOrder = ProfileIsMixedProbeOrder =
709+
true;
707710
break;
708711
case SecNameTable: {
709712
bool FixedLengthMD5 =
@@ -1369,6 +1372,8 @@ static std::string getSecFlagsStr(const SecHdrTableEntry &Entry) {
13691372
Flags.append("preInlined,");
13701373
if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagFSDiscriminator))
13711374
Flags.append("fs-discriminator,");
1375+
if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagIsMixedProbeOrder))
1376+
Flags.append("mixed-probe-order,");
13721377
break;
13731378
case SecFuncOffsetTable:
13741379
if (hasSecFlag(Entry, SecFuncOffsetFlags::SecFlagOrdered))

llvm/lib/ProfileData/SampleProfWriter.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,10 @@ std::error_code SampleProfileWriterExtBinaryBase::writeOneSection(
437437
addSectionFlag(SecProfSummary, SecProfSummaryFlags::SecFlagIsPreInlined);
438438
if (Type == SecProfSummary && FunctionSamples::ProfileIsFS)
439439
addSectionFlag(SecProfSummary, SecProfSummaryFlags::SecFlagFSDiscriminator);
440+
if (Type == SecProfSummary && FunctionSamples::ProfileIsProbeBased &&
441+
FunctionSamples::ProfileIsMixedProbeOrder)
442+
addSectionFlag(SecProfSummary,
443+
SecProfSummaryFlags::SecFlagIsMixedProbeOrder);
440444

441445
uint64_t SectionStart = markSectionStart(Type, LayoutIdx);
442446
switch (Type) {

llvm/lib/Transforms/IPO/SampleProfile.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1129,7 +1129,7 @@ void SampleProfileLoader::findExternalInlineCandidate(
11291129
CalleeSample->getContext().hasAttribute(ContextShouldBeInlined);
11301130
if (!PreInline && CalleeSample->getHeadSamplesEstimate() < Threshold)
11311131
continue;
1132-
1132+
11331133
Function *Func = SymbolMap.lookup(CalleeSample->getFunction());
11341134
// Add to the import list only when it's defined out of module.
11351135
if (!Func || Func->isDeclaration())
@@ -2112,6 +2112,14 @@ bool SampleProfileLoader::doInitialization(Module &M,
21122112
DS_Warning));
21132113
return false;
21142114
}
2115+
2116+
if (!FunctionSamples::ProfileIsMixedProbeOrder) {
2117+
const char *Msg =
2118+
"Pseudo-probe-based profile is on an old version ID order which "
2119+
"could cause profile mismatch(performance regression)";
2120+
Ctx.diagnose(DiagnosticInfoSampleProfile(M.getModuleIdentifier(), Msg,
2121+
DS_Warning));
2122+
}
21152123
}
21162124

21172125
if (ReportProfileStaleness || PersistProfileStaleness ||

llvm/tools/llvm-profgen/ProfileGenerator.cpp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,54 @@ void ProfileGenerator::populateBodySamplesWithProbesForAllFunctions(
559559
}
560560
}
561561

562+
// Use a heuristic to determine probe order by checking callsite insertion
563+
// position relative to the BB probes. Sort all the BB probes is in a list, for
564+
// each calliste, compute "ratio = insert position / length of the list". For
565+
// the old order, the probe ids for BB should be all before(smaller than) the
566+
// probe ids for callsite, this ratio should be equal to or close to 1.
567+
inline bool checkProbeIDIsInMixedOrder(const SampleProfileMap &Profiles) {
568+
// Use flattned profile to maximize the callsites in the profile.
569+
SampleProfileMap flattenProfile;
570+
ProfileConverter::flattenProfile(Profiles, flattenProfile);
571+
572+
uint32_t PossibleOldOrderNum = 0;
573+
uint32_t PossibleNewOrderNum = 0;
574+
575+
for (const auto &I : flattenProfile) {
576+
const FunctionSamples &FS = I.second;
577+
// Skip small functions whose profile order are likely random.
578+
if (FS.getBodySamples().size() < 10)
579+
continue;
580+
581+
std::set<uint32_t> PossibleBBProbeIDs;
582+
std::set<uint32_t> CallsiteIDs;
583+
for (const auto &I : FS.getBodySamples()) {
584+
if (I.second.getCallTargets().empty())
585+
PossibleBBProbeIDs.insert(I.first.LineOffset);
586+
else
587+
CallsiteIDs.insert(I.first.LineOffset);
588+
}
589+
590+
if (PossibleBBProbeIDs.empty() || CallsiteIDs.empty())
591+
continue;
592+
593+
uint32_t DistanceToBeginSum = 0;
594+
for (const auto &ID : CallsiteIDs)
595+
DistanceToBeginSum += std::distance(PossibleBBProbeIDs.begin(),
596+
PossibleBBProbeIDs.upper_bound(ID));
597+
uint32_t LengthSum = PossibleBBProbeIDs.size() * CallsiteIDs.size();
598+
599+
// Note that PossibleBBProbeIDs could contains some callsite probe id, the
600+
// ratio is not exactly 1 for the old order, hence use a smaller threshold
601+
// to determine.
602+
if ((float)DistanceToBeginSum / LengthSum > 0.8)
603+
PossibleOldOrderNum++;
604+
else
605+
PossibleNewOrderNum++;
606+
}
607+
return PossibleNewOrderNum >= PossibleOldOrderNum;
608+
}
609+
562610
void ProfileGenerator::populateBoundarySamplesWithProbesForAllFunctions(
563611
const BranchSample &BranchCounters) {
564612
for (const auto &Entry : BranchCounters) {
@@ -1024,6 +1072,16 @@ void CSProfileGenerator::postProcessProfiles() {
10241072
CSConverter.convertCSProfiles();
10251073
FunctionSamples::ProfileIsCS = false;
10261074
}
1075+
1076+
if (FunctionSamples::ProfileIsProbeBased) {
1077+
FunctionSamples::ProfileIsMixedProbeOrder = true;
1078+
if (!checkProbeIDIsInMixedOrder(ProfileMap)) {
1079+
WithColor::warning()
1080+
<< "Pseudo-probe-based profile is on an old version ID order which "
1081+
"could cause profile mismatch(performance regression)\n";
1082+
FunctionSamples::ProfileIsMixedProbeOrder = false;
1083+
}
1084+
}
10271085
}
10281086

10291087
void ProfileGeneratorBase::computeSummaryAndThreshold(

0 commit comments

Comments
 (0)