Skip to content

Commit 5e6b16f

Browse files
committed
Detect the order and add a summary flag to the profile
1 parent e6f735a commit 5e6b16f

File tree

7 files changed

+91
-2
lines changed

7 files changed

+91
-2
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: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2173,6 +2173,14 @@ bool SampleProfileLoader::doInitialization(Module &M,
21732173
DS_Warning));
21742174
return false;
21752175
}
2176+
2177+
if (!FunctionSamples::ProfileIsMixedProbeOrder) {
2178+
const char *Msg =
2179+
"Pseudo-probe-based profile is on an old version ID order which "
2180+
"could cause profile mismatch(performance regression)";
2181+
Ctx.diagnose(DiagnosticInfoSampleProfile(M.getModuleIdentifier(), Msg,
2182+
DS_Warning));
2183+
}
21762184
}
21772185

21782186
if (ReportProfileStaleness || PersistProfileStaleness ||

llvm/tools/llvm-profgen/ProfileGenerator.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,66 @@ ProfileGenerator::getTopLevelFunctionProfile(FunctionId FuncName) {
513513
return ProfileMap.Create(Context);
514514
}
515515

516+
// Use a heuristic to determine probe order by checking callsite insertion
517+
// position relative to the BB probes. Sort all the BB probes is in a list, for
518+
// each calliste, compute "ratio = insert position / length of the list". For
519+
// the old order, the probe ids for BB should be all before(smaller than) the
520+
// probe ids for callsite, this ratio should be equal to or close to 1.
521+
bool checkProbeIDIsInMixedOrder(const SampleProfileMap &Profiles) {
522+
// Use flattned profile to maximize the callsites in the profile.
523+
SampleProfileMap flattenProfile;
524+
ProfileConverter::flattenProfile(Profiles, flattenProfile);
525+
526+
uint32_t PossibleOldOrderNum = 0;
527+
uint32_t PossibleNewOrderNum = 0;
528+
529+
for (const auto &I : flattenProfile) {
530+
const FunctionSamples &FS = I.second;
531+
// Skip small functions whose profile order are likely random.
532+
if (FS.getBodySamples().size() < 10)
533+
continue;
534+
535+
std::set<uint32_t> PossibleBBProbeIDs;
536+
std::set<uint32_t> CallsiteIDs;
537+
for (const auto &I : FS.getBodySamples()) {
538+
if (I.second.getCallTargets().empty())
539+
PossibleBBProbeIDs.insert(I.first.LineOffset);
540+
else
541+
CallsiteIDs.insert(I.first.LineOffset);
542+
}
543+
544+
if (PossibleBBProbeIDs.empty() || CallsiteIDs.empty())
545+
continue;
546+
547+
uint32_t DistanceToBeginSum = 0;
548+
for (const auto &ID : CallsiteIDs)
549+
DistanceToBeginSum += std::distance(PossibleBBProbeIDs.begin(),
550+
PossibleBBProbeIDs.upper_bound(ID));
551+
uint32_t LengthSum = PossibleBBProbeIDs.size() * CallsiteIDs.size();
552+
553+
// Note that PossibleBBProbeIDs could contains some callsite probe id, the
554+
// ratio is not exactly 1 for the old order, hence use a smaller threshold
555+
// to determine.
556+
if ((float)DistanceToBeginSum / LengthSum > 0.8)
557+
PossibleOldOrderNum++;
558+
else
559+
PossibleNewOrderNum++;
560+
}
561+
return PossibleNewOrderNum >= PossibleOldOrderNum;
562+
}
563+
564+
void warnOrWriteProbeOrderFlag(const SampleProfileMap &Profiles) {
565+
if (FunctionSamples::ProfileIsProbeBased) {
566+
FunctionSamples::ProfileIsMixedProbeOrder = true;
567+
if (!checkProbeIDIsInMixedOrder(Profiles)) {
568+
WithColor::warning()
569+
<< "Pseudo-probe-based profile is on an old version ID order which "
570+
"could cause profile mismatch(performance regression)\n";
571+
FunctionSamples::ProfileIsMixedProbeOrder = false;
572+
}
573+
}
574+
}
575+
516576
void ProfileGenerator::generateProfile() {
517577
collectProfiledFunctions();
518578

@@ -535,6 +595,7 @@ void ProfileGenerator::postProcessProfiles() {
535595
trimColdProfiles(ProfileMap, ColdCountThreshold);
536596
filterAmbiguousProfile(ProfileMap);
537597
calculateAndShowDensity(ProfileMap);
598+
warnOrWriteProbeOrderFlag(ProfileMap);
538599
}
539600

540601
void ProfileGenerator::trimColdProfiles(const SampleProfileMap &Profiles,
@@ -1068,6 +1129,7 @@ void CSProfileGenerator::postProcessProfiles() {
10681129
FunctionSamples::ProfileIsCS = false;
10691130
}
10701131
filterAmbiguousProfile(ProfileMap);
1132+
warnOrWriteProbeOrderFlag(ProfileMap);
10711133
}
10721134

10731135
void ProfileGeneratorBase::computeSummaryAndThreshold(

0 commit comments

Comments
 (0)