Skip to content

Commit 1dafad1

Browse files
committed
Revert "[llvm-profgen] Support creating profiles of arbitrary events (llvm#99026)"
This reverts commit 0caf0c9.
1 parent 52fad27 commit 1dafad1

File tree

9 files changed

+20
-331
lines changed

9 files changed

+20
-331
lines changed
Binary file not shown.

llvm/test/tools/llvm-profgen/Inputs/cmov_3.perfscript

Lines changed: 0 additions & 39 deletions
This file was deleted.

llvm/test/tools/llvm-profgen/Inputs/ip-duplication.perfscript

Lines changed: 0 additions & 2 deletions
This file was deleted.

llvm/test/tools/llvm-profgen/Inputs/noprobe-skid.perfscript

Lines changed: 0 additions & 5 deletions
This file was deleted.

llvm/test/tools/llvm-profgen/event-filtering.test

Lines changed: 0 additions & 78 deletions
This file was deleted.

llvm/test/tools/llvm-profgen/iponly-nodupfactor.test

Lines changed: 0 additions & 22 deletions
This file was deleted.

llvm/test/tools/llvm-profgen/iponly.test

Lines changed: 0 additions & 58 deletions
This file was deleted.

llvm/tools/llvm-profgen/PerfReader.cpp

Lines changed: 9 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,6 @@ static cl::opt<bool>
4141
"and produce context-insensitive profile."));
4242
cl::opt<bool> ShowDetailedWarning("show-detailed-warning",
4343
cl::desc("Show detailed warning message."));
44-
cl::opt<bool>
45-
LeadingIPOnly("leading-ip-only",
46-
cl::desc("Form a profile based only on sample IPs"));
47-
48-
static cl::list<std::string> PerfEventFilter(
49-
"perf-event",
50-
cl::desc("Ignore samples not matching the given event names"));
51-
static cl::alias
52-
PerfEventFilterPlural("perf-events", cl::CommaSeparated,
53-
cl::desc("Comma-delimited version of -perf-event"),
54-
cl::aliasopt(PerfEventFilter));
5544

5645
extern cl::opt<std::string> PerfTraceFilename;
5746
extern cl::opt<bool> ShowDisassemblyOnly;
@@ -415,18 +404,13 @@ PerfScriptReader::convertPerfDataToTrace(ProfiledBinary *Binary, bool SkipPID,
415404
}
416405
}
417406

418-
// If filtering by events was requested, additionally request the "event"
419-
// field.
420-
const std::string FieldList =
421-
PerfEventFilter.empty() ? "ip,brstack" : "event,ip,brstack";
422-
423407
// Run perf script again to retrieve events for PIDs collected above
424408
SmallVector<StringRef, 8> ScriptSampleArgs;
425409
ScriptSampleArgs.push_back(PerfPath);
426410
ScriptSampleArgs.push_back("script");
427411
ScriptSampleArgs.push_back("--show-mmap-events");
428412
ScriptSampleArgs.push_back("-F");
429-
ScriptSampleArgs.push_back(FieldList);
413+
ScriptSampleArgs.push_back("ip,brstack");
430414
ScriptSampleArgs.push_back("-i");
431415
ScriptSampleArgs.push_back(PerfData);
432416
if (!PIDs.empty()) {
@@ -591,54 +575,14 @@ bool PerfScriptReader::extractLBRStack(TraceStream &TraceIt,
591575

592576
// Skip the leading instruction pointer.
593577
size_t Index = 0;
594-
595-
StringRef EventName;
596-
// Skip a perf event name. This may or may not exist.
597-
if (Records.size() > Index && Records[Index].ends_with(":")) {
598-
EventName = Records[Index].ltrim().rtrim(':');
599-
Index++;
600-
601-
if (PerfEventFilter.empty()) {
602-
WithColor::warning() << "No --perf-event filter was specified, but an "
603-
"\"event\" field was found in line "
604-
<< TraceIt.getLineNumber() << ": "
605-
<< TraceIt.getCurrentLine() << "\n";
606-
} else if (std::find(PerfEventFilter.begin(), PerfEventFilter.end(),
607-
EventName) == PerfEventFilter.end()) {
608-
TraceIt.advance();
609-
return false;
610-
}
611-
612-
} else if (!PerfEventFilter.empty()) {
613-
WithColor::warning() << "A --perf-event filter was specified, but no "
614-
"\"event\" field found in line "
615-
<< TraceIt.getLineNumber() << ": "
616-
<< TraceIt.getCurrentLine() << "\n";
617-
}
618-
619578
uint64_t LeadingAddr;
620-
if (Records.size() > Index && !Records[Index].contains('/')) {
621-
if (Records[Index].getAsInteger(16, LeadingAddr)) {
579+
if (!Records.empty() && !Records[0].contains('/')) {
580+
if (Records[0].getAsInteger(16, LeadingAddr)) {
622581
WarnInvalidLBR(TraceIt);
623582
TraceIt.advance();
624583
return false;
625584
}
626-
Index++;
627-
}
628-
629-
// We assume that if we saw an event name we also saw a leading addr.
630-
// In other words, LeadingAddr is set if Index is 1 or 2.
631-
if (LeadingIPOnly && Index > 0) {
632-
// Form a profile only from the sample IP. Do not assume an LBR stack
633-
// follows, and ignore it if it does.
634-
uint64_t SampleIP = Binary->canonicalizeVirtualAddress(LeadingAddr);
635-
bool SampleIPIsInternal = Binary->addressIsCode(SampleIP);
636-
if (SampleIPIsInternal) {
637-
// Form a half LBR entry where the sample IP is the destination.
638-
LBRStack.emplace_back(LBREntry(SampleIP, SampleIP));
639-
}
640-
TraceIt.advance();
641-
return !LBRStack.empty();
585+
Index = 1;
642586
}
643587

644588
// Now extract LBR samples - note that we do not reverse the
@@ -958,20 +902,6 @@ void PerfScriptReader::computeCounterFromLBR(const PerfSample *Sample,
958902
uint64_t Repeat) {
959903
SampleCounter &Counter = SampleCounters.begin()->second;
960904
uint64_t EndAddress = 0;
961-
962-
if (LeadingIPOnly) {
963-
assert(Sample->LBRStack.size() == 1 &&
964-
"Expected only half LBR entries for ip-only mode");
965-
const LBREntry &LBR = *(Sample->LBRStack.begin());
966-
uint64_t SourceAddress = LBR.Source;
967-
uint64_t TargetAddress = LBR.Target;
968-
if (SourceAddress == TargetAddress &&
969-
Binary->addressIsCode(TargetAddress)) {
970-
Counter.recordRangeCount(SourceAddress, TargetAddress, Repeat);
971-
}
972-
return;
973-
}
974-
975905
for (const LBREntry &LBR : Sample->LBRStack) {
976906
uint64_t SourceAddress = LBR.Source;
977907
uint64_t TargetAddress = LBR.Target;
@@ -1132,18 +1062,6 @@ bool PerfScriptReader::isLBRSample(StringRef Line) {
11321062
Line.trim().split(Records, " ", 2, false);
11331063
if (Records.size() < 2)
11341064
return false;
1135-
// Check if there is an event name before the leading IP.
1136-
// If there is, it will be in Records[0]. To skip it, we'll re-split on
1137-
// Records[1], which should contain the rest of the line.
1138-
if (Records[0].contains(":")) {
1139-
// If so, consume the event name and continue processing the rest of the
1140-
// line.
1141-
StringRef IPAndLBR = Records[1].ltrim();
1142-
Records.clear();
1143-
IPAndLBR.split(Records, " ", 2, false);
1144-
if (Records.size() < 2)
1145-
return false;
1146-
}
11471065
if (Records[1].starts_with("0x") && Records[1].contains('/'))
11481066
return true;
11491067
return false;
@@ -1234,18 +1152,6 @@ void PerfScriptReader::warnInvalidRange() {
12341152
const PerfSample *Sample = Item.first.getPtr();
12351153
uint64_t Count = Item.second;
12361154
uint64_t EndAddress = 0;
1237-
1238-
if (LeadingIPOnly) {
1239-
assert(Sample->LBRStack.size() == 1 &&
1240-
"Expected only half LBR entries for ip-only mode");
1241-
const LBREntry &LBR = *(Sample->LBRStack.begin());
1242-
if (LBR.Source == LBR.Target && LBR.Source != ExternalAddr) {
1243-
// This is an leading-addr-only profile.
1244-
Ranges[{LBR.Source, LBR.Source}] += Count;
1245-
}
1246-
continue;
1247-
}
1248-
12491155
for (const LBREntry &LBR : Sample->LBRStack) {
12501156
uint64_t SourceAddress = LBR.Source;
12511157
uint64_t StartAddress = LBR.Target;
@@ -1293,15 +1199,11 @@ void PerfScriptReader::warnInvalidRange() {
12931199
!Binary->addressIsCode(EndAddress))
12941200
continue;
12951201

1296-
// IP samples can indicate activity on individual instructions rather than
1297-
// basic blocks/edges. In this mode, don't warn if sampled IPs aren't
1298-
// branches.
1299-
if (!LeadingIPOnly)
1300-
if (!Binary->addressIsCode(StartAddress) ||
1301-
!Binary->addressIsTransfer(EndAddress)) {
1302-
InstNotBoundary += I.second;
1303-
WarnInvalidRange(StartAddress, EndAddress, EndNotBoundaryMsg);
1304-
}
1202+
if (!Binary->addressIsCode(StartAddress) ||
1203+
!Binary->addressIsTransfer(EndAddress)) {
1204+
InstNotBoundary += I.second;
1205+
WarnInvalidRange(StartAddress, EndAddress, EndNotBoundaryMsg);
1206+
}
13051207

13061208
auto *FRange = Binary->findFuncRange(StartAddress);
13071209
if (!FRange) {

llvm/tools/llvm-profgen/ProfileGenerator.cpp

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,6 @@ cl::opt<bool> InferMissingFrames(
104104
"Infer missing call frames due to compiler tail call elimination."),
105105
llvm::cl::Optional);
106106

107-
extern cl::opt<bool> LeadingIPOnly;
108-
109107
using namespace llvm;
110108
using namespace sampleprof;
111109

@@ -390,25 +388,18 @@ void ProfileGeneratorBase::updateBodySamplesforFunctionProfile(
390388
// Use the maximum count of samples with same line location
391389
uint32_t Discriminator = getBaseDiscriminator(LeafLoc.Location.Discriminator);
392390

393-
if (LeadingIPOnly) {
394-
// When computing an IP-based profile we take the SUM of counts at the
395-
// location instead of applying duplication factors and taking the MAX.
391+
// Use duplication factor to compensated for loop unroll/vectorization.
392+
// Note that this is only needed when we're taking MAX of the counts at
393+
// the location instead of SUM.
394+
Count *= getDuplicationFactor(LeafLoc.Location.Discriminator);
395+
396+
ErrorOr<uint64_t> R =
397+
FunctionProfile.findSamplesAt(LeafLoc.Location.LineOffset, Discriminator);
398+
399+
uint64_t PreviousCount = R ? R.get() : 0;
400+
if (PreviousCount <= Count) {
396401
FunctionProfile.addBodySamples(LeafLoc.Location.LineOffset, Discriminator,
397-
Count);
398-
} else {
399-
// Otherwise, use duplication factor to compensate for loop
400-
// unroll/vectorization. Note that this is only needed when we're taking
401-
// MAX of the counts at the location instead of SUM.
402-
Count *= getDuplicationFactor(LeafLoc.Location.Discriminator);
403-
404-
ErrorOr<uint64_t> R = FunctionProfile.findSamplesAt(
405-
LeafLoc.Location.LineOffset, Discriminator);
406-
407-
uint64_t PreviousCount = R ? R.get() : 0;
408-
if (PreviousCount <= Count) {
409-
FunctionProfile.addBodySamples(LeafLoc.Location.LineOffset, Discriminator,
410-
Count - PreviousCount);
411-
}
402+
Count - PreviousCount);
412403
}
413404
}
414405

0 commit comments

Comments
 (0)