[llvm] 2cd20c2 - Revert "[llvm-exegesis] Add support for pinning benchmarking process to a CPU (#85168)"

Aiden Grossman via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 23 09:09:06 PDT 2024


Author: Aiden Grossman
Date: 2024-09-23T16:08:14Z
New Revision: 2cd20c255684257b86940bdda6861897f0bf3c00

URL: https://github.com/llvm/llvm-project/commit/2cd20c255684257b86940bdda6861897f0bf3c00
DIFF: https://github.com/llvm/llvm-project/commit/2cd20c255684257b86940bdda6861897f0bf3c00.diff

LOG: Revert "[llvm-exegesis] Add support for pinning benchmarking process to a CPU (#85168)"

This reverts commit 6fc2451167ec991361dd0568de9a9fa2926f8da8.

This broke some more buildbots.

Added: 
    

Modified: 
    llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
    llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
    llvm/tools/llvm-exegesis/llvm-exegesis.cpp

Removed: 
    llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning-execution-mode.s
    llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning.s


################################################################################
diff  --git a/llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning-execution-mode.s b/llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning-execution-mode.s
deleted file mode 100644
index b73ac26f2cfc74..00000000000000
--- a/llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning-execution-mode.s
+++ /dev/null
@@ -1,5 +0,0 @@
-# REQUIRES: exegesis-can-measure-latency, x86_64-linux
-
-# RUN: not llvm-exegesis -mtriple=x86_64-unknown-unknown -mode=latency -opcode-name=ADD64rr -execution-mode=inprocess --benchmark-process-cpu=0 2>&1 | FileCheck %s
-
-# CHECK: llvm-exegesis error: The inprocess execution mode does not support benchmark core pinning.

diff  --git a/llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning.s b/llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning.s
deleted file mode 100644
index 0ea3752fc3bb95..00000000000000
--- a/llvm/test/tools/llvm-exegesis/X86/latency/cpu-pinning.s
+++ /dev/null
@@ -1,5 +0,0 @@
-# REQUIRES: exegesis-can-measure-latency, x86_64-linux
-
-# RUN: llvm-exegesis -mtriple=x86_64-unknown-unknown -mode=latency -opcode-name=ADD64rr -execution-mode=subprocess | FileCheck %s
-
-# CHECK: - { key: latency, value: {{[0-9.]*}}, per_snippet_value: {{[0-9.]*}}

diff  --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index 27ad7508756f10..4e60d33fa2c637 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -98,8 +98,7 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
 public:
   static Expected<std::unique_ptr<InProcessFunctionExecutorImpl>>
   create(const LLVMState &State, object::OwningBinary<object::ObjectFile> Obj,
-         BenchmarkRunner::ScratchSpace *Scratch,
-         std::optional<int> BenchmarkProcessCPU) {
+         BenchmarkRunner::ScratchSpace *Scratch) {
     Expected<ExecutableFunction> EF =
         ExecutableFunction::create(State.createTargetMachine(), std::move(Obj));
 
@@ -191,31 +190,27 @@ class SubProcessFunctionExecutorImpl
 public:
   static Expected<std::unique_ptr<SubProcessFunctionExecutorImpl>>
   create(const LLVMState &State, object::OwningBinary<object::ObjectFile> Obj,
-         const BenchmarkKey &Key, std::optional<int> BenchmarkProcessCPU) {
+         const BenchmarkKey &Key) {
     Expected<ExecutableFunction> EF =
         ExecutableFunction::create(State.createTargetMachine(), std::move(Obj));
     if (!EF)
       return EF.takeError();
 
     return std::unique_ptr<SubProcessFunctionExecutorImpl>(
-        new SubProcessFunctionExecutorImpl(State, std::move(*EF), Key,
-                                           BenchmarkProcessCPU));
+        new SubProcessFunctionExecutorImpl(State, std::move(*EF), Key));
   }
 
 private:
   SubProcessFunctionExecutorImpl(const LLVMState &State,
                                  ExecutableFunction Function,
-                                 const BenchmarkKey &Key,
-                                 std::optional<int> BenchmarkCPU)
-      : State(State), Function(std::move(Function)), Key(Key),
-        BenchmarkProcessCPU(BenchmarkCPU) {}
+                                 const BenchmarkKey &Key)
+      : State(State), Function(std::move(Function)), Key(Key) {}
 
   enum ChildProcessExitCodeE {
     CounterFDReadFailed = 1,
     RSeqDisableFailed,
     FunctionDataMappingFailed,
-    AuxiliaryMemorySetupFailed,
-    SetCPUAffinityFailed
+    AuxiliaryMemorySetupFailed
   };
 
   StringRef childProcessExitCodeToString(int ExitCode) const {
@@ -228,8 +223,6 @@ class SubProcessFunctionExecutorImpl
       return "Failed to map memory for assembled snippet";
     case ChildProcessExitCodeE::AuxiliaryMemorySetupFailed:
       return "Failed to setup auxiliary memory";
-    case ChildProcessExitCodeE::SetCPUAffinityFailed:
-      return "Failed to set CPU affinity of the benchmarking process";
     default:
       return "Child process returned with unknown exit code";
     }
@@ -391,36 +384,6 @@ class SubProcessFunctionExecutorImpl
     return make_error<SnippetSignal>(ChildSignalInfo.si_signo);
   }
 
-  static void setCPUAffinityIfRequested(int CPUToUse) {
-// Special case this function for x86_64 for now as certain more esoteric
-// platforms have 
diff erent definitions for some of the libc functions that
-// cause buildtime failures. Additionally, the subprocess executor mode (the
-// sole mode where this is supported) currently only supports x86_64.
-#if defined(__x86_64__)
-    // Set the CPU affinity for the child process, so that we ensure that if
-    // the user specified a CPU the process should run on, the benchmarking
-    // process is running on that CPU.
-    cpu_set_t CPUMask;
-    CPU_ZERO(&CPUMask);
-    CPU_SET(CPUToUse, &CPUMask);
-    // TODO(boomanaiden154): Rewrite this to use LLVM primitives once they
-    // are available.
-    int SetAffinityReturn = sched_setaffinity(0, sizeof(CPUMask), &CPUMask);
-    if (SetAffinityReturn == -1) {
-      exit(ChildProcessExitCodeE::SetCPUAffinityFailed);
-    }
-
-    // Check (if assertions are enabled) that we are actually running on the
-    // CPU that was specified by the user.
-    [[maybe_unused]] unsigned int CurrentCPU;
-    assert(getcpu(&CurrentCPU, nullptr) == 0 &&
-           "Expected getcpu call to succeed.");
-    assert(static_cast<int>(CurrentCPU) == CPUToUse &&
-           "Expected current CPU to equal the CPU requested by the user");
-#endif // defined(__x86_64__)
-    exit(ChildProcessExitCodeE::SetCPUAffinityFailed);
-  }
-
   Error createSubProcessAndRunBenchmark(
       StringRef CounterName, SmallVectorImpl<int64_t> &CounterValues,
       ArrayRef<const char *> ValidationCounters,
@@ -453,10 +416,6 @@ class SubProcessFunctionExecutorImpl
     }
 
     if (ParentOrChildPID == 0) {
-      if (BenchmarkProcessCPU.has_value()) {
-        setCPUAffinityIfRequested(*BenchmarkProcessCPU);
-      }
-
       // 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
@@ -579,7 +538,6 @@ class SubProcessFunctionExecutorImpl
   const LLVMState &State;
   const ExecutableFunction Function;
   const BenchmarkKey &Key;
-  const std::optional<int> BenchmarkProcessCPU;
 };
 #endif // __linux__
 } // namespace
@@ -657,15 +615,11 @@ BenchmarkRunner::getRunnableConfiguration(
 Expected<std::unique_ptr<BenchmarkRunner::FunctionExecutor>>
 BenchmarkRunner::createFunctionExecutor(
     object::OwningBinary<object::ObjectFile> ObjectFile,
-    const BenchmarkKey &Key, std::optional<int> BenchmarkProcessCPU) const {
+    const BenchmarkKey &Key) const {
   switch (ExecutionMode) {
   case ExecutionModeE::InProcess: {
-    if (BenchmarkProcessCPU.has_value())
-      return make_error<Failure>("The inprocess execution mode does not "
-                                 "support benchmark core pinning.");
-
     auto InProcessExecutorOrErr = InProcessFunctionExecutorImpl::create(
-        State, std::move(ObjectFile), Scratch.get(), BenchmarkProcessCPU);
+        State, std::move(ObjectFile), Scratch.get());
     if (!InProcessExecutorOrErr)
       return InProcessExecutorOrErr.takeError();
 
@@ -674,7 +628,7 @@ BenchmarkRunner::createFunctionExecutor(
   case ExecutionModeE::SubProcess: {
 #ifdef __linux__
     auto SubProcessExecutorOrErr = SubProcessFunctionExecutorImpl::create(
-        State, std::move(ObjectFile), Key, BenchmarkProcessCPU);
+        State, std::move(ObjectFile), Key);
     if (!SubProcessExecutorOrErr)
       return SubProcessExecutorOrErr.takeError();
 
@@ -689,8 +643,8 @@ BenchmarkRunner::createFunctionExecutor(
 }
 
 std::pair<Error, Benchmark> BenchmarkRunner::runConfiguration(
-    RunnableConfiguration &&RC, const std::optional<StringRef> &DumpFile,
-    std::optional<int> BenchmarkProcessCPU) const {
+    RunnableConfiguration &&RC,
+    const std::optional<StringRef> &DumpFile) const {
   Benchmark &BenchmarkResult = RC.BenchmarkResult;
   object::OwningBinary<object::ObjectFile> &ObjectFile = RC.ObjectFile;
 
@@ -711,8 +665,7 @@ std::pair<Error, Benchmark> BenchmarkRunner::runConfiguration(
   }
 
   Expected<std::unique_ptr<BenchmarkRunner::FunctionExecutor>> Executor =
-      createFunctionExecutor(std::move(ObjectFile), RC.BenchmarkResult.Key,
-                             BenchmarkProcessCPU);
+      createFunctionExecutor(std::move(ObjectFile), RC.BenchmarkResult.Key);
   if (!Executor)
     return {Executor.takeError(), std::move(BenchmarkResult)};
   auto NewMeasurements = runMeasurements(**Executor);

diff  --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
index e688b814d1c83d..9b4bb1d41149fe 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
@@ -68,8 +68,7 @@ class BenchmarkRunner {
 
   std::pair<Error, Benchmark>
   runConfiguration(RunnableConfiguration &&RC,
-                   const std::optional<StringRef> &DumpFile,
-                   std::optional<int> BenchmarkProcessCPU) const;
+                   const std::optional<StringRef> &DumpFile) const;
 
   // Scratch space to run instructions that touch memory.
   struct ScratchSpace {
@@ -136,8 +135,7 @@ class BenchmarkRunner {
 
   Expected<std::unique_ptr<FunctionExecutor>>
   createFunctionExecutor(object::OwningBinary<object::ObjectFile> Obj,
-                         const BenchmarkKey &Key,
-                         std::optional<int> BenchmarkProcessCPU) const;
+                         const BenchmarkKey &Key) const;
 };
 
 } // namespace exegesis

diff  --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index 546ec770a8d221..e6a43cfc6db51c 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -269,11 +269,6 @@ static cl::list<ValidationEvent> ValidationCounters(
         "counter to validate benchmarking assumptions"),
     cl::CommaSeparated, cl::cat(BenchmarkOptions), ValidationEventOptions());
 
-static cl::opt<int> BenchmarkProcessCPU(
-    "benchmark-process-cpu",
-    cl::desc("The CPU number that the benchmarking process should executon on"),
-    cl::cat(BenchmarkOptions), cl::init(-1));
-
 static ExitOnError ExitOnErr("llvm-exegesis error: ");
 
 // Helper function that logs the error(s) and exits.
@@ -423,12 +418,8 @@ static void runBenchmarkConfigurations(
         std::optional<StringRef> DumpFile;
         if (DumpObjectToDisk.getNumOccurrences())
           DumpFile = DumpObjectToDisk;
-        const std::optional<int> BenchmarkCPU =
-            BenchmarkProcessCPU == -1
-                ? std::nullopt
-                : std::optional(BenchmarkProcessCPU.getValue());
         auto [Err, BenchmarkResult] =
-            Runner.runConfiguration(std::move(RC), DumpFile, BenchmarkCPU);
+            Runner.runConfiguration(std::move(RC), DumpFile);
         if (Err) {
           // Errors from executing the snippets are fine.
           // All other errors are a framework issue and should fail.


        


More information about the llvm-commits mailing list