Skip to content

Commit 6fe180e

Browse files
committed
Revert "Detect the order and add a summary flag to the profile"
This reverts commit 28b0374.
1 parent 5e6b16f commit 6fe180e

File tree

7 files changed

+3
-92
lines changed

7 files changed

+3
-92
lines changed

llvm/include/llvm/ProfileData/SampleProf.h

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,6 @@ 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),
208204
};
209205

210206
enum class SecFuncMetadataFlags : uint32_t {
@@ -470,7 +466,7 @@ struct SampleContextFrame {
470466
LineLocation Location;
471467

472468
SampleContextFrame() : Location(0, 0) {}
473-
469+
474470
SampleContextFrame(FunctionId Func, LineLocation Location)
475471
: Func(Func), Location(Location) {}
476472

@@ -531,7 +527,7 @@ class SampleContext {
531527
: Func(Name), State(UnknownContext), Attributes(ContextNone) {
532528
assert(!Name.empty() && "Name is empty");
533529
}
534-
530+
535531
SampleContext(FunctionId Func)
536532
: Func(Func), State(UnknownContext), Attributes(ContextNone) {}
537533

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

11831179
static bool ProfileIsProbeBased;
11841180

1185-
static bool ProfileIsMixedProbeOrder;
1186-
11871181
static bool ProfileIsCS;
11881182

11891183
static bool ProfileIsPreInlined;

llvm/include/llvm/ProfileData/SampleProfReader.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -525,9 +525,6 @@ 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-
531528
/// Whether function profiles are context-sensitive flat profiles.
532529
bool ProfileIsCS = false;
533530

llvm/lib/ProfileData/SampleProf.cpp

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

llvm/lib/ProfileData/SampleProfReader.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -704,9 +704,6 @@ 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;
710707
break;
711708
case SecNameTable: {
712709
bool FixedLengthMD5 =
@@ -1372,8 +1369,6 @@ static std::string getSecFlagsStr(const SecHdrTableEntry &Entry) {
13721369
Flags.append("preInlined,");
13731370
if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagFSDiscriminator))
13741371
Flags.append("fs-discriminator,");
1375-
if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagIsMixedProbeOrder))
1376-
Flags.append("mixed-probe-order,");
13771372
break;
13781373
case SecFuncOffsetTable:
13791374
if (hasSecFlag(Entry, SecFuncOffsetFlags::SecFlagOrdered))

llvm/lib/ProfileData/SampleProfWriter.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,10 +437,6 @@ 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);
444440

445441
uint64_t SectionStart = markSectionStart(Type, LayoutIdx);
446442
switch (Type) {

llvm/lib/Transforms/IPO/SampleProfile.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,7 +1190,7 @@ void SampleProfileLoader::findExternalInlineCandidate(
11901190
CalleeSample->getContext().hasAttribute(ContextShouldBeInlined);
11911191
if (!PreInline && CalleeSample->getHeadSamplesEstimate() < Threshold)
11921192
continue;
1193-
1193+
11941194
Function *Func = SymbolMap.lookup(CalleeSample->getFunction());
11951195
// Add to the import list only when it's defined out of module.
11961196
if (!Func || Func->isDeclaration())
@@ -2173,14 +2173,6 @@ 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-
}
21842176
}
21852177

21862178
if (ReportProfileStaleness || PersistProfileStaleness ||

llvm/tools/llvm-profgen/ProfileGenerator.cpp

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -513,66 +513,6 @@ 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-
576516
void ProfileGenerator::generateProfile() {
577517
collectProfiledFunctions();
578518

@@ -595,7 +535,6 @@ void ProfileGenerator::postProcessProfiles() {
595535
trimColdProfiles(ProfileMap, ColdCountThreshold);
596536
filterAmbiguousProfile(ProfileMap);
597537
calculateAndShowDensity(ProfileMap);
598-
warnOrWriteProbeOrderFlag(ProfileMap);
599538
}
600539

601540
void ProfileGenerator::trimColdProfiles(const SampleProfileMap &Profiles,
@@ -1129,7 +1068,6 @@ void CSProfileGenerator::postProcessProfiles() {
11291068
FunctionSamples::ProfileIsCS = false;
11301069
}
11311070
filterAmbiguousProfile(ProfileMap);
1132-
warnOrWriteProbeOrderFlag(ProfileMap);
11331071
}
11341072

11351073
void ProfileGeneratorBase::computeSummaryAndThreshold(

0 commit comments

Comments
 (0)