[llvm-branch-commits] [llvm] 66873fb - [CSSPGO][llvm-profgen] Renovate perfscript check and command line input validation
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Feb 19 21:28:56 PST 2021
Author: wlei
Date: 2021-02-19T21:21:13-08:00
New Revision: 66873fb695370f5bd333e327ec77e4710c7891c2
URL: https://github.com/llvm/llvm-project/commit/66873fb695370f5bd333e327ec77e4710c7891c2
DIFF: https://github.com/llvm/llvm-project/commit/66873fb695370f5bd333e327ec77e4710c7891c2.diff
LOG: [CSSPGO][llvm-profgen] Renovate perfscript check and command line input validation
This include some changes related with PerfReader's the input check and command line change:
1) It appears there might be thousands of leading MMAP-Event line in the perfscript for large workload. For this case, the 4k threshold is not eligible to determine it's a hybrid sample. This change renovated the `isHybridPerfScript` by going through the script without threshold limitation checking whether there is a non-empty call stack immediately followed by a LBR sample. It will stop once it find a valid one.
2) Added several input validations for the command line switches in PerfReader.
3) Changed the command line `show-disassembly` to `show-disassembly-only`, it will print to stdout and exit early which leave an empty output profile.
Reviewed By: hoy, wenlei
Differential Revision: https://reviews.llvm.org/D96387
Added:
llvm/test/tools/llvm-profgen/invalid-perfscript.test
Modified:
llvm/test/tools/llvm-profgen/disassemble.s
llvm/test/tools/llvm-profgen/pseudoprobe-decoding.test
llvm/test/tools/llvm-profgen/symbolize.ll
llvm/tools/llvm-profgen/PerfReader.cpp
llvm/tools/llvm-profgen/PerfReader.h
llvm/tools/llvm-profgen/ProfileGenerator.cpp
llvm/tools/llvm-profgen/ProfiledBinary.cpp
llvm/tools/llvm-profgen/llvm-profgen.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-profgen/disassemble.s b/llvm/test/tools/llvm-profgen/disassemble.s
index fc85fbe967e0..be03b5a6892b 100644
--- a/llvm/test/tools/llvm-profgen/disassemble.s
+++ b/llvm/test/tools/llvm-profgen/disassemble.s
@@ -1,6 +1,6 @@
# REQUIRES: x86-registered-target
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
-# RUN: llvm-profgen --binary=%t --perfscript=%s --output=%t1 -show-disassembly -x86-asm-syntax=intel | FileCheck %s --match-full-lines
+# RUN: llvm-profgen --binary=%t --perfscript=%s --output=%t1 -show-disassembly-only -x86-asm-syntax=intel | FileCheck %s --match-full-lines
# CHECK: Disassembly of section .text [0x0, 0x66]:
# CHECK: <foo1>:
diff --git a/llvm/test/tools/llvm-profgen/invalid-perfscript.test b/llvm/test/tools/llvm-profgen/invalid-perfscript.test
new file mode 100644
index 000000000000..d795f85b1ea3
--- /dev/null
+++ b/llvm/test/tools/llvm-profgen/invalid-perfscript.test
@@ -0,0 +1,9 @@
+; RUN: llvm-profgen --perfscript=%s --binary=%S/Inputs/noinline-cs-noprobe.perfbin --output=%t 2>%t1
+; RUN: FileCheck %s --input-file %t1
+
+ 4005dc
+ 400634
+ 400684
+ 7f68c5788793
+
+; XFAIL: *
diff --git a/llvm/test/tools/llvm-profgen/pseudoprobe-decoding.test b/llvm/test/tools/llvm-profgen/pseudoprobe-decoding.test
index 5feaa97032ab..1d93a06d8e42 100644
--- a/llvm/test/tools/llvm-profgen/pseudoprobe-decoding.test
+++ b/llvm/test/tools/llvm-profgen/pseudoprobe-decoding.test
@@ -1,4 +1,4 @@
-; RUN: llvm-profgen --perfscript=%s --binary=%S/Inputs/inline-cs-pseudoprobe.perfbin --output=%t --show-pseudo-probe --show-disassembly | FileCheck %s
+; RUN: llvm-profgen --perfscript=%s --binary=%S/Inputs/inline-cs-pseudoprobe.perfbin --output=%t --show-pseudo-probe --show-disassembly-only | FileCheck %s
PERF_RECORD_MMAP2 2854748/2854748: [0x400000(0x1000) @ 0 00:1d 123291722 526021]: r-xp /home/inline-cs-pseudoprobe.perfbin
diff --git a/llvm/test/tools/llvm-profgen/symbolize.ll b/llvm/test/tools/llvm-profgen/symbolize.ll
index 2fbc59e3d00d..9a436dec4c20 100644
--- a/llvm/test/tools/llvm-profgen/symbolize.ll
+++ b/llvm/test/tools/llvm-profgen/symbolize.ll
@@ -1,6 +1,6 @@
; REQUIRES: x86-registered-target
; RUN: llc -filetype=obj %s -o %t
-; RUN: llvm-profgen --binary=%t --perfscript=%s --output=%t1 --show-disassembly -x86-asm-syntax=intel --show-source-locations | FileCheck %s --match-full-lines
+; RUN: llvm-profgen --binary=%t --perfscript=%s --output=%t1 --show-disassembly-only -x86-asm-syntax=intel --show-source-locations | FileCheck %s --match-full-lines
; CHECK: Disassembly of section .text [0x0, 0x4a]:
; CHECK: <funcA>:
diff --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp
index e59d8d93381b..2e0b71f38e6d 100644
--- a/llvm/tools/llvm-profgen/PerfReader.cpp
+++ b/llvm/tools/llvm-profgen/PerfReader.cpp
@@ -17,6 +17,9 @@ static cl::opt<bool> ShowUnwinderOutput("show-unwinder-output",
cl::ZeroOrMore,
cl::desc("Print unwinder output"));
+extern cl::opt<bool> ShowDisassemblyOnly;
+extern cl::opt<bool> ShowSourceLocations;
+
namespace llvm {
namespace sampleprof {
@@ -230,7 +233,44 @@ bool VirtualUnwinder::unwind(const HybridSample *Sample, uint64_t Repeat) {
return true;
}
-PerfReader::PerfReader(cl::list<std::string> &BinaryFilenames) {
+void PerfReader::validateCommandLine(
+ cl::list<std::string> &BinaryFilenames,
+ cl::list<std::string> &PerfTraceFilenames) {
+ // Allow the invalid perfscript if we only use to show binary disassembly
+ if (!ShowDisassemblyOnly) {
+ for (auto &File : PerfTraceFilenames) {
+ if (!llvm::sys::fs::exists(File)) {
+ std::string Msg = "Input perf script(" + File + ") doesn't exist!";
+ exitWithError(Msg);
+ }
+ }
+ }
+ if (BinaryFilenames.size() > 1) {
+ // TODO: remove this if everything is ready to support multiple binaries.
+ exitWithError(
+ "Currently only support one input binary, multiple binaries' "
+ "profile will be merged in one profile and make profile "
+ "summary info inaccurate. Please use `llvm-perfdata` to merge "
+ "profiles from multiple binaries.");
+ }
+ for (auto &Binary : BinaryFilenames) {
+ if (!llvm::sys::fs::exists(Binary)) {
+ std::string Msg = "Input binary(" + Binary + ") doesn't exist!";
+ exitWithError(Msg);
+ }
+ }
+ if (CSProfileGenerator::MaxCompressionSize < -1) {
+ exitWithError("Value of --compress-recursion should >= -1");
+ }
+ if (ShowSourceLocations && !ShowDisassemblyOnly) {
+ exitWithError("--show-source-locations should work together with "
+ "--show-disassembly-only!");
+ }
+}
+
+PerfReader::PerfReader(cl::list<std::string> &BinaryFilenames,
+ cl::list<std::string> &PerfTraceFilenames) {
+ validateCommandLine(BinaryFilenames, PerfTraceFilenames);
// Load the binaries.
for (auto Filename : BinaryFilenames)
loadBinary(Filename, /*AllowNameConflict*/ false);
@@ -591,27 +631,13 @@ void PerfReader::parseAndAggregateTrace(StringRef Filename) {
void PerfReader::checkAndSetPerfType(
cl::list<std::string> &PerfTraceFilenames) {
- bool HasHybridPerf = true;
for (auto FileName : PerfTraceFilenames) {
- if (!isHybridPerfScript(FileName)) {
- HasHybridPerf = false;
- break;
- }
- }
-
- if (HasHybridPerf) {
- PerfType = PERF_LBR_STACK;
- } else {
- // TODO: Support other type of perf script
- PerfType = PERF_INVILID;
- }
-
- if (BinaryTable.size() > 1) {
- // TODO: remove this if everything is ready to support multiple binaries.
- exitWithError("Currently only support one input binary, multiple binaries' "
- "profile will be merged in one profile and make profile "
- "summary info inaccurate. Please use `perfdata` to merge "
- "profiles from multiple binaries.");
+ PerfScriptType Type = checkPerfScriptType(FileName);
+ if (Type == PERF_INVALID)
+ exitWithError("Invalid perf script input!");
+ if (PerfType != PERF_UNKNOWN && PerfType != Type)
+ exitWithError("Inconsistent sample among
diff erent perf scripts");
+ PerfType = Type;
}
}
diff --git a/llvm/tools/llvm-profgen/PerfReader.h b/llvm/tools/llvm-profgen/PerfReader.h
index 7eaa4b846259..b802c212eb46 100644
--- a/llvm/tools/llvm-profgen/PerfReader.h
+++ b/llvm/tools/llvm-profgen/PerfReader.h
@@ -59,9 +59,10 @@ class TraceStream {
// The type of perfscript
enum PerfScriptType {
- PERF_INVILID = 0,
- PERF_LBR = 1, // Only LBR sample
- PERF_LBR_STACK = 2, // Hybrid sample including call stack and LBR stack.
+ PERF_UNKNOWN = 0,
+ PERF_INVALID = 1,
+ PERF_LBR = 2, // Only LBR sample
+ PERF_LBR_STACK = 3, // Hybrid sample including call stack and LBR stack.
};
// The parsed LBR sample entry.
@@ -502,19 +503,52 @@ using BinarySampleCounterMap =
class PerfReader {
public:
- PerfReader(cl::list<std::string> &BinaryFilenames);
-
- // Hybrid sample(call stack + LBRs) profile traces are seprated by double line
- // break, search for that within the first 4k charactors to avoid going
- // through the whole file.
- static bool isHybridPerfScript(StringRef FileName) {
- auto BufOrError = MemoryBuffer::getFileOrSTDIN(FileName, 4000);
- if (!BufOrError)
- exitWithError(BufOrError.getError(), FileName);
- auto Buffer = std::move(BufOrError.get());
- if (Buffer->getBuffer().find("\n\n") == StringRef::npos)
+ PerfReader(cl::list<std::string> &BinaryFilenames,
+ cl::list<std::string> &PerfTraceFilenames);
+
+ // A LBR sample is like:
+ // 0x5c6313f/0x5c63170/P/-/-/0 0x5c630e7/0x5c63130/P/-/-/0 ...
+ // A heuristic for fast detection by checking whether a
+ // leading " 0x" and the '/' exist.
+ static bool isLBRSample(StringRef Line) {
+ if (!Line.startswith(" 0x"))
return false;
- return true;
+ if (Line.find('/') != StringRef::npos)
+ return true;
+ return false;
+ }
+
+ // The raw hybird sample is like
+ // e.g.
+ // 4005dc # call stack leaf
+ // 400634
+ // 400684 # call stack root
+ // 0x4005c8/0x4005dc/P/-/-/0 0x40062f/0x4005b0/P/-/-/0 ...
+ // ... 0x4005c8/0x4005dc/P/-/-/0 # LBR Entries
+ // Determine the perfscript contains hybrid samples(call stack + LBRs) by
+ // checking whether there is a non-empty call stack immediately followed by
+ // a LBR sample
+ static PerfScriptType checkPerfScriptType(StringRef FileName) {
+ TraceStream TraceIt(FileName);
+ uint64_t FrameAddr = 0;
+ while (!TraceIt.isAtEoF()) {
+ int32_t Count = 0;
+ while (!TraceIt.isAtEoF() &&
+ !TraceIt.getCurrentLine().ltrim().getAsInteger(16, FrameAddr)) {
+ Count++;
+ TraceIt.advance();
+ }
+ if (!TraceIt.isAtEoF()) {
+ if (isLBRSample(TraceIt.getCurrentLine())) {
+ if (Count > 0)
+ return PERF_LBR_STACK;
+ else
+ return PERF_LBR;
+ }
+ TraceIt.advance();
+ }
+ }
+ return PERF_INVALID;
}
// The parsed MMap event
@@ -540,6 +574,9 @@ class PerfReader {
}
private:
+ /// Validate the command line input
+ void validateCommandLine(cl::list<std::string> &BinaryFilenames,
+ cl::list<std::string> &PerfTraceFilenames);
/// Parse a single line of a PERF_RECORD_MMAP2 event looking for a
/// mapping between the binary name and its memory layout.
///
@@ -574,7 +611,7 @@ class PerfReader {
BinarySampleCounterMap BinarySampleCounters;
// Samples with the repeating time generated by the perf reader
AggregatedCounter AggregatedSamples;
- PerfScriptType PerfType;
+ PerfScriptType PerfType = PERF_UNKNOWN;
};
} // end namespace sampleprof
diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
index 0a7dddc06bfc..553ea71ea1fc 100644
--- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp
+++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
@@ -11,6 +11,8 @@
static cl::opt<std::string> OutputFilename("output", cl::value_desc("output"),
cl::Required,
cl::desc("Output profile file"));
+static cl::alias OutputA("o", cl::desc("Alias for --output"),
+ cl::aliasopt(OutputFilename));
static cl::opt<SampleProfileFormat> OutputFormat(
"format", cl::desc("Format of output profile"), cl::init(SPF_Text),
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
index e1549b14bf05..d7588d680cca 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
@@ -22,16 +22,15 @@
using namespace llvm;
using namespace sampleprof;
-static cl::opt<bool> ShowDisassembly("show-disassembly", cl::ReallyHidden,
- cl::init(false), cl::ZeroOrMore,
- cl::desc("Print disassembled code."));
+cl::opt<bool> ShowDisassemblyOnly("show-disassembly-only", cl::ReallyHidden,
+ cl::init(false), cl::ZeroOrMore,
+ cl::desc("Print disassembled code."));
-static cl::opt<bool> ShowSourceLocations("show-source-locations",
- cl::ReallyHidden, cl::init(false),
- cl::ZeroOrMore,
- cl::desc("Print source locations."));
+cl::opt<bool> ShowSourceLocations("show-source-locations", cl::ReallyHidden,
+ cl::init(false), cl::ZeroOrMore,
+ cl::desc("Print source locations."));
-static cl::opt<bool> ShowPseudoProbe(
+cl::opt<bool> ShowPseudoProbe(
"show-pseudo-probe", cl::ReallyHidden, cl::init(false), cl::ZeroOrMore,
cl::desc("Print pseudo probe section and disassembled info."));
@@ -199,7 +198,6 @@ void ProfiledBinary::decodePseudoProbe(const ELFObjectFileBase *Obj) {
bool ProfiledBinary::dissassembleSymbol(std::size_t SI, ArrayRef<uint8_t> Bytes,
SectionSymbolsTy &Symbols,
const SectionRef &Section) {
-
std::size_t SE = Symbols.size();
uint64_t SectionOffset = Section.getAddress() - PreferredBaseAddress;
uint64_t SectSize = Section.getSize();
@@ -211,7 +209,7 @@ bool ProfiledBinary::dissassembleSymbol(std::size_t SI, ArrayRef<uint8_t> Bytes,
return true;
std::string &&SymbolName = Symbols[SI].Name.str();
- if (ShowDisassembly)
+ if (ShowDisassemblyOnly)
outs() << '<' << SymbolName << ">:\n";
uint64_t Offset = StartOffset;
@@ -223,7 +221,7 @@ bool ProfiledBinary::dissassembleSymbol(std::size_t SI, ArrayRef<uint8_t> Bytes,
Offset + PreferredBaseAddress, nulls()))
return false;
- if (ShowDisassembly) {
+ if (ShowDisassemblyOnly) {
if (ShowPseudoProbe) {
ProbeDecoder.printProbeForAddress(outs(),
Offset + PreferredBaseAddress);
@@ -257,7 +255,7 @@ bool ProfiledBinary::dissassembleSymbol(std::size_t SI, ArrayRef<uint8_t> Bytes,
Offset += Size;
}
- if (ShowDisassembly)
+ if (ShowDisassemblyOnly)
outs() << "\n";
FuncStartAddrMap[StartOffset] = Symbols[SI].Name.str();
@@ -323,7 +321,7 @@ void ProfiledBinary::disassemble(const ELFObjectFileBase *Obj) {
for (std::pair<const SectionRef, SectionSymbolsTy> &SecSyms : AllSymbols)
stable_sort(SecSyms.second);
- if (ShowDisassembly)
+ if (ShowDisassemblyOnly)
outs() << "\nDisassembly of " << FileName << ":\n";
// Dissassemble a text section.
@@ -342,7 +340,7 @@ void ProfiledBinary::disassemble(const ELFObjectFileBase *Obj) {
// Register the text section.
TextSections.insert({SectionOffset, SectSize});
- if (ShowDisassembly) {
+ if (ShowDisassemblyOnly) {
StringRef SectionName = unwrapOrError(Section.getName(), FileName);
outs() << "\nDisassembly of section " << SectionName;
outs() << " [" << format("0x%" PRIx64, SectionOffset) << ", "
diff --git a/llvm/tools/llvm-profgen/llvm-profgen.cpp b/llvm/tools/llvm-profgen/llvm-profgen.cpp
index 0f4d8f015439..081f1bb4fcf4 100644
--- a/llvm/tools/llvm-profgen/llvm-profgen.cpp
+++ b/llvm/tools/llvm-profgen/llvm-profgen.cpp
@@ -29,6 +29,8 @@ static cl::list<std::string>
llvm::cl::MiscFlags::CommaSeparated,
cl::desc("Path of profiled binary files"));
+extern cl::opt<bool> ShowDisassemblyOnly;
+
using namespace llvm;
using namespace sampleprof;
@@ -43,7 +45,9 @@ int main(int argc, const char *argv[]) {
cl::ParseCommandLineOptions(argc, argv, "llvm SPGO profile generator\n");
// Load binaries and parse perf events and samples
- PerfReader Reader(BinaryFilenames);
+ PerfReader Reader(BinaryFilenames, PerfTraceFilenames);
+ if (ShowDisassemblyOnly)
+ return EXIT_SUCCESS;
Reader.parsePerfTraces(PerfTraceFilenames);
std::unique_ptr<ProfileGenerator> Generator = ProfileGenerator::create(
More information about the llvm-branch-commits
mailing list