Revert "[llvm-exegesis] Add ability to assign perf counters to specific PID"

boomanaiden154 · boomanaiden154 · commit 309950515c3e · 2023-06-22T18:05:01.000Z
Revert "[llvm-exegesis] Introduce Subprocess Executor Mode" This reverts commit 5e9173c. This reverts commit 4d618b5. Reverting the PID commit as it is currently breaking MinGW builds and the way I'm checking for the presence of pid_t needs to be fixed and I need to do some testing. The subprocess executor mode patch is a dependent patch so also needs to be reverted and also needs some work as it is currently failing tests where libpfm is installed and the kernel version is less than 5.6.
diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-abnormal-exit-code.s b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-abnormal-exit-code.s
diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-segfault.s b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-segfault.s
diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess.s b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess.s
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -25,18 +25,6 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Program.h"
-#include "llvm/Support/Signals.h"
-
-#ifdef __linux__
-#ifdef HAVE_LIBPFM
-#include <perfmon/perf_event.h>
-#endif
-#include <sys/mman.h>
-#include <sys/ptrace.h>
-#include <sys/syscall.h>
-#include <sys/wait.h>
-#include <unistd.h>
-#endif // __linux__
 
 namespace llvm {
 namespace exegesis {
@@ -141,182 +129,6 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
   const ExecutableFunction Function;
   BenchmarkRunner::ScratchSpace *const Scratch;
 };
-
-#ifdef __linux__
-// The following class implements a function executor that executes the
-// benchmark code within a subprocess rather than within the main llvm-exegesis
-// process. This allows for much more control over the execution context of the
-// snippet, particularly with regard to memory. This class performs all the
-// necessary functions to create the subprocess, execute the snippet in the
-// subprocess, and report results/handle errors.
-class SubProcessFunctionExecutorImpl
-    : public BenchmarkRunner::FunctionExecutor {
-public:
-  SubProcessFunctionExecutorImpl(const LLVMState &State,
-                                 object::OwningBinary<object::ObjectFile> Obj,
-                                 const BenchmarkKey &Key)
-      : State(State), Function(State.createTargetMachine(), std::move(Obj)),
-        Key(Key) {}
-
-private:
-  enum ChildProcessExitCodeE {
-    CounterFDReadFailed = 1,
-    TranslatingCounterFDFailed
-  };
-
-  StringRef childProcessExitCodeToString(int ExitCode) const {
-    switch (ExitCode) {
-    case ChildProcessExitCodeE::CounterFDReadFailed:
-      return "Counter file descriptor read failed";
-    case ChildProcessExitCodeE::TranslatingCounterFDFailed:
-      return "Translating counter file descriptor into a file descriptor in "
-             "the child process failed. This might be due running an older "
-             "Linux kernel that doesn't support the pidfd_getfd system call "
-             "(anything before Linux 5.6).";
-    default:
-      return "Child process returned with unknown exit code";
-    }
-  }
-
-  Error createSubProcessAndRunBenchmark(
-      StringRef CounterName, SmallVectorImpl<int64_t> &CounterValues) const {
-    int PipeFiles[2];
-    int PipeSuccessOrErr = pipe(PipeFiles);
-    if (PipeSuccessOrErr != 0) {
-      return make_error<Failure>(
-          "Failed to create a pipe for interprocess communication between "
-          "llvm-exegesis and the benchmarking subprocess");
-    }
-
-    pid_t ParentOrChildPID = fork();
-    if (ParentOrChildPID == 0) {
-      // We are in the child process, close the write end of the pipe
-      close(PipeFiles[1]);
-      // Unregister handlers, signal handling is now handled through ptrace in
-      // the host process
-      llvm::sys::unregisterHandlers();
-      prepareAndRunBenchmark(PipeFiles[0], Key);
-      // The child process terminates in the above function, so we should never
-      // get to this point.
-      llvm_unreachable("Child process didn't exit when expected.");
-    }
-
-    const ExegesisTarget &ET = State.getExegesisTarget();
-    auto CounterOrError =
-        ET.createCounter(CounterName, State, ParentOrChildPID);
-
-    if (!CounterOrError)
-      return CounterOrError.takeError();
-
-    pfm::Counter *Counter = CounterOrError.get().get();
-
-    close(PipeFiles[0]);
-
-    int CounterFileDescriptor = Counter->getFileDescriptor();
-    ssize_t BytesWritten =
-        write(PipeFiles[1], &CounterFileDescriptor, sizeof(int));
-
-    if (BytesWritten != sizeof(int))
-      return make_error<Failure>("Writing peformance counter file descriptor "
-                                 "to child process failed: " +
-                                 Twine(strerror(errno)));
-
-    if (ptrace(PTRACE_SEIZE, ParentOrChildPID, NULL, NULL) != 0)
-      return make_error<Failure>("Failed to seize the child process: " +
-                                 Twine(strerror(errno)));
-
-    int ChildStatus;
-    if (wait(&ChildStatus) == -1) {
-      return make_error<Failure>(
-          "Waiting for the child process to complete failed: " +
-          Twine(strerror(errno)));
-    }
-
-    if (WIFEXITED(ChildStatus)) {
-      int ChildExitCode = WEXITSTATUS(ChildStatus);
-      if (ChildExitCode == 0) {
-        // The child exited succesfully, read counter values and return
-        // success
-        CounterValues[0] = Counter->read();
-        return Error::success();
-      }
-      // The child exited, but not successfully
-      return make_error<SnippetCrash>(
-          "Child benchmarking process exited with non-zero exit code: " +
-          childProcessExitCodeToString(ChildExitCode));
-    }
-
-    // An error was encountered running the snippet, process it
-    siginfo_t ChildSignalInfo;
-    if (ptrace(PTRACE_GETSIGINFO, ParentOrChildPID, NULL, &ChildSignalInfo) ==
-        -1) {
-      return make_error<Failure>("Getting signal info from the child failed: " +
-                                 Twine(strerror(errno)));
-    }
-
-    return make_error<SnippetCrash>(
-        "The benchmarking subprocess sent unexpected signal: " +
-        Twine(strsignal(ChildSignalInfo.si_signo)));
-  }
-
-  [[noreturn]] void prepareAndRunBenchmark(int Pipe,
-                                           const BenchmarkKey &Key) const {
-    // The following occurs within the benchmarking subprocess
-
-    int ParentCounterFileDescriptor = -1;
-    ssize_t BytesRead = read(Pipe, &ParentCounterFileDescriptor, sizeof(int));
-
-    if (BytesRead != sizeof(int)) {
-      exit(ChildProcessExitCodeE::CounterFDReadFailed);
-    }
-
-    // Make sure the following two syscalls are defined on the platform that
-    // we're building on as they were introduced to the kernel fairly recently
-    // (v5.6 for the second one).
-#if defined SYS_pidfd_open && defined SYS_pidfd_getfd
-    pid_t ParentPID = getppid();
-
-    int ParentPIDFD = syscall(SYS_pidfd_open, ParentPID, 0);
-    int CounterFileDescriptor =
-        syscall(SYS_pidfd_getfd, ParentPIDFD, ParentCounterFileDescriptor, 0);
-#else
-    int CounterFileDescriptor = 0;
-    exit(ChildProcessExitCodeE::TranslatingCounterFDFailed);
-#endif
-
-    if (CounterFileDescriptor == -1) {
-      exit(ChildProcessExitCodeE::TranslatingCounterFDFailed);
-    }
-
-#ifdef HAVE_LIBPFM
-    ioctl(CounterFileDescriptor, PERF_EVENT_IOC_RESET);
-#endif
-    this->Function(nullptr);
-#ifdef HAVE_LIBPFM
-    ioctl(CounterFileDescriptor, PERF_EVENT_IOC_DISABLE);
-#endif
-
-    exit(0);
-  }
-
-  Expected<llvm::SmallVector<int64_t, 4>>
-  runWithCounter(StringRef CounterName) const override {
-    SmallVector<int64_t, 4> Value(1, 0);
-    Error PossibleBenchmarkError =
-        createSubProcessAndRunBenchmark(CounterName, Value);
-
-    if (PossibleBenchmarkError) {
-      return std::move(PossibleBenchmarkError);
-    }
-
-    return Value;
-  }
-
-  const LLVMState &State;
-  const ExecutableFunction Function;
-  const BenchmarkKey &Key;
-};
-#endif // __linux__
 } // namespace
 
 Expected<SmallString<0>> BenchmarkRunner::assembleSnippet(
@@ -389,14 +201,6 @@ BenchmarkRunner::createFunctionExecutor(
   case ExecutionModeE::InProcess:
     return std::make_unique<InProcessFunctionExecutorImpl>(
         State, std::move(ObjectFile), Scratch.get());
-  case ExecutionModeE::SubProcess:
-#ifdef __linux__
-    return std::make_unique<SubProcessFunctionExecutorImpl>(
-        State, std::move(ObjectFile), Key);
-#else
-    return make_error<Failure>(
-        "The subprocess execution mode is only supported on Linux");
-#endif
   }
   llvm_unreachable("ExecutionMode is outside expected range");
 }
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
@@ -34,7 +34,7 @@ namespace exegesis {
 // Common code for all benchmark modes.
 class BenchmarkRunner {
 public:
-  enum ExecutionModeE { InProcess, SubProcess };
+  enum ExecutionModeE { InProcess };
 
   explicit BenchmarkRunner(const LLVMState &State, Benchmark::ModeE Mode,
                            BenchmarkPhaseSelectorE BenchmarkPhaseSelector,
diff --git a/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp b/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp
@@ -107,20 +107,21 @@ StringRef PerfEvent::getPfmEventString() const {
   return FullQualifiedEventString;
 }
 
-Counter::Counter(PerfEvent &&E, pid_t ProcessID) : Event(std::move(E)) {
+Counter::Counter(PerfEvent &&E) : Event(std::move(E)){
   assert(Event.valid());
   IsDummyEvent = Event.name() == PerfEvent::DummyEventString;
   if (!IsDummyEvent)
-    initRealEvent(E, ProcessID);
+    initRealEvent(E);
 }
 
 #ifdef HAVE_LIBPFM
-void Counter::initRealEvent(const PerfEvent &E, pid_t ProcessID) {
+void Counter::initRealEvent(const PerfEvent &E) {
+  const pid_t Pid = 0;    // measure current process/thread.
   const int Cpu = -1;     // measure any processor.
   const int GroupFd = -1; // no grouping of counters.
   const uint32_t Flags = 0;
   perf_event_attr AttrCopy = *Event.attribute();
-  FileDescriptor = perf_event_open(&AttrCopy, ProcessID, Cpu, GroupFd, Flags);
+  FileDescriptor = perf_event_open(&AttrCopy, Pid, Cpu, GroupFd, Flags);
   if (FileDescriptor == -1) {
     errs() << "Unable to open event. ERRNO: " << strerror(errno)
            << ". Make sure your kernel allows user "
@@ -179,7 +180,7 @@ Counter::readOrError(StringRef /*unused*/) const {
 int Counter::numValues() const { return 1; }
 #else
 
-void Counter::initRealEvent(const PerfEvent &, pid_t ProcessID) {}
+void Counter::initRealEvent(const PerfEvent &) {}
 
 Counter::~Counter() = default;
 
diff --git a/llvm/tools/llvm-exegesis/lib/PerfHelper.h b/llvm/tools/llvm-exegesis/lib/PerfHelper.h
@@ -23,10 +23,6 @@
 #include <functional>
 #include <memory>
 
-#ifndef HAVE_LIBPFM
-typedef int pid_t;
-#endif // HAVE_LIBPFM
-
 struct perf_event_attr;
 
 namespace llvm {
@@ -80,7 +76,7 @@ class PerfEvent {
 class Counter {
 public:
   // event: the PerfEvent to measure.
-  explicit Counter(PerfEvent &&event, pid_t ProcessID = 0);
+  explicit Counter(PerfEvent &&event);
 
   Counter(const Counter &) = delete;
   Counter(Counter &&other) = default;
@@ -107,15 +103,15 @@ class Counter {
 
   virtual int numValues() const;
 
-  int getFileDescriptor() const { return FileDescriptor; }
-
 protected:
   PerfEvent Event;
+#ifdef HAVE_LIBPFM
   int FileDescriptor = -1;
+#endif
   bool IsDummyEvent;
 
 private:
-  void initRealEvent(const PerfEvent &E, pid_t ProcessID);
+  void initRealEvent(const PerfEvent &E);
 };
 
 } // namespace pfm
diff --git a/llvm/tools/llvm-exegesis/lib/Target.cpp b/llvm/tools/llvm-exegesis/lib/Target.cpp
@@ -35,16 +35,15 @@ const ExegesisTarget *ExegesisTarget::lookup(Triple TT) {
 }
 
 Expected<std::unique_ptr<pfm::Counter>>
-ExegesisTarget::createCounter(StringRef CounterName, const LLVMState &,
-                              const pid_t ProcessID) const {
+ExegesisTarget::createCounter(StringRef CounterName, const LLVMState &) const {
   pfm::PerfEvent Event(CounterName);
   if (!Event.valid())
     return llvm::make_error<Failure>(
         llvm::Twine("Unable to create counter with name '")
             .concat(CounterName)
             .concat("'"));
 
-  return std::make_unique<pfm::Counter>(std::move(Event), ProcessID);
+  return std::make_unique<pfm::Counter>(std::move(Event));
 }
 
 void ExegesisTarget::registerTarget(ExegesisTarget *Target) {
diff --git a/llvm/tools/llvm-exegesis/lib/Target.h b/llvm/tools/llvm-exegesis/lib/Target.h
@@ -75,8 +75,7 @@ class ExegesisTarget {
 
   // Targets can use this to create target-specific perf counters.
   virtual Expected<std::unique_ptr<pfm::Counter>>
-  createCounter(StringRef CounterName, const LLVMState &State,
-                const pid_t ProcessID = 0) const;
+  createCounter(StringRef CounterName, const LLVMState &State) const;
 
   // Targets can use this to add target-specific passes in assembleToStream();
   virtual void addTargetSpecificPasses(PassManagerBase &PM) const {}
diff --git a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
@@ -665,8 +665,7 @@ class ExegesisX86Target : public ExegesisTarget {
   ExegesisX86Target() : ExegesisTarget(X86CpuPfmCounters) {}
 
   Expected<std::unique_ptr<pfm::Counter>>
-  createCounter(StringRef CounterName, const LLVMState &State,
-                const pid_t ProcessID) const override {
+  createCounter(StringRef CounterName, const LLVMState &State) const override {
     // If LbrSamplingPeriod was provided, then ignore the
     // CounterName because we only have one for LBR.
     if (LbrSamplingPeriod > 0) {
@@ -683,7 +682,7 @@ class ExegesisX86Target : public ExegesisTarget {
           llvm::errc::invalid_argument);
 #endif
     }
-    return ExegesisTarget::createCounter(CounterName, State, ProcessID);
+    return ExegesisTarget::createCounter(CounterName, State);
   }
 
 private:
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -254,11 +254,7 @@ static cl::opt<BenchmarkRunner::ExecutionModeE> ExecutionMode(
     cl::cat(BenchmarkOptions),
     cl::values(clEnumValN(BenchmarkRunner::ExecutionModeE::InProcess,
                           "inprocess",
-                          "Executes the snippets within the same process"),
-               clEnumValN(BenchmarkRunner::ExecutionModeE::SubProcess,
-                          "subprocess",
-                          "Spawns a subprocess for each snippet execution, "
-                          "allows for the use of memory annotations")),
+                          "Executes the snippets within the same process")),
     cl::init(BenchmarkRunner::ExecutionModeE::InProcess));
 
 static ExitOnError ExitOnErr("llvm-exegesis error: ");
