[PATCH] D89723: [CSSPGO][llvm-profgen]Context-sensitive profile data generation

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 20 13:53:58 PST 2020


hoy added inline comments.


================
Comment at: llvm/test/tools/llvm-profgen/Inputs/noinline-cs-noprobe.perfscript:2
+Using perf wrapper that supports hot-text. Try perf.real if you encounter any issues.
+PERF_RECORD_MMAP2 2854748/2854748: [0x400000(0x1000) @ 0 00:1d 123291722 526021]: r-xp /home/noinline-cs-noprobe.perfbin
+
----------------
wlei wrote:
> wenlei wrote:
> > I think we also need to support cases where PERF_RECORD_MMAP2 event isn't available, in which case we just use preferred load address from ELF header. 
> > 
> > Can you add a test case that doesn't have PERF_RECORD_MMAP2? Looks like currently we would just proceed with parsing without a base address set?
> Yeah, currently PERF_RECORD_MMAP2 is required.
> The problem using preferred load address for non-mmap event is one perf address might belong to multiple binaries, which will mess up the whole process. Also we need to one more perftrace scan to confirm there is no mmap2 event so that we can switch to use preferred address.
> or we can have a switch like "--no-mmp2-events" to explicitly tell the tool use preferred address, also only support one binary under this switch. or we need some info in the perf trace tell which binary it belong to(I remembered we discuss this internally). any suggestion on this?
Maybe the binary lookup table can be pre-filled with preferred load address when the binary is loaded/constructed. Without mmap2 events in the trace file, subsequent processing with just use the preferred addresses.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:47
+void ProfileGenerator::write() {
+  auto WriterOrErr = SampleProfileWriter::create(OutputFilename, OutputFormat);
+  if (std::error_code EC = WriterOrErr.getError())
----------------
wlei wrote:
> hoy wrote:
> > I'm wondering if a separate profile file should be output for each binary. Since the samples are already separated for binaries via `BinarySampleCounters`,  `ProfileMap` can be made like that too.
> Yeah, it's doable. but that needs more CL design, currently we only support one output file, so we have to change supporting multiple output files which also need an exact one-one mapping to the binary. So we can use `OutputFilenames` to receives multiple output files and match them in order on the command line? or I'm also thinking we just remain this and if the user really need to separate the output for binary, they could call the tool multiple times with different input binary. any suggestions on the command?
I see. Let's keep a single output for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89723/new/

https://reviews.llvm.org/D89723



More information about the llvm-commits mailing list