[llvm] d228059 - [llvm-exegesis] Refactor common parts out of FunctionExecutorImpl

[Aiden Grossman via llvm-commits]

Author: Aiden Grossman
Date: 2023-04-14T07:37:28Z
New Revision: d22805940a8ba42f6ff3b1cd2e549e9d54e40248

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

LOG: [llvm-exegesis] Refactor common parts out of FunctionExecutorImpl

This patch refactors some code out of FunctionExecutorImpl into the base
class that should be common across all implementations of
FunctionExecutor. Particularly, this patch factors out
accumulateCounterValues, and also factors out runAndSample, moving
implementation specific code into a new runWithCounter function. This
makes adding new implementations of FunctinExecutor easier.

Reviewed By: gchatelet

Differential Revision: https://reviews.llvm.org/D148079

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index b0ea639ba656..b747d2351ba1 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -37,6 +37,34 @@ BenchmarkRunner::BenchmarkRunner(const LLVMState &State,
 
 BenchmarkRunner::~BenchmarkRunner() = default;
 
+void BenchmarkRunner::FunctionExecutor::accumulateCounterValues(
+    const llvm::SmallVectorImpl<int64_t> &NewValues,
+    llvm::SmallVectorImpl<int64_t> *Result) {
+  const size_t NumValues = std::max(NewValues.size(), Result->size());
+  if (NumValues > Result->size())
+    Result->resize(NumValues, 0);
+  for (size_t I = 0, End = NewValues.size(); I < End; ++I)
+    (*Result)[I] += NewValues[I];
+}
+
+Expected<llvm::SmallVector<int64_t, 4>>
+BenchmarkRunner::FunctionExecutor::runAndSample(const char *Counters) const {
+  // We sum counts when there are several counters for a single ProcRes
+  // (e.g. P23 on SandyBridge).
+  llvm::SmallVector<int64_t, 4> CounterValues;
+  SmallVector<StringRef, 2> CounterNames;
+  StringRef(Counters).split(CounterNames, '+');
+  for (auto &CounterName : CounterNames) {
+    CounterName = CounterName.trim();
+    Expected<SmallVector<int64_t, 4>> ValueOrError =
+        runWithCounter(CounterName);
+    if (!ValueOrError)
+      return ValueOrError.takeError();
+    accumulateCounterValues(ValueOrError.get(), &CounterValues);
+  }
+  return CounterValues;
+};
+
 namespace {
 class FunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
 public:
@@ -58,67 +86,43 @@ class FunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
   }
 
   Expected<llvm::SmallVector<int64_t, 4>>
-  runAndSample(const char *Counters) const override {
-    // We sum counts when there are several counters for a single ProcRes
-    // (e.g. P23 on SandyBridge).
-    llvm::SmallVector<int64_t, 4> CounterValues;
-    int Reserved = 0;
-    SmallVector<StringRef, 2> CounterNames;
-    StringRef(Counters).split(CounterNames, '+');
-    char *const ScratchPtr = Scratch->ptr();
+  runWithCounter(StringRef CounterName) const override {
     const ExegesisTarget &ET = State.getExegesisTarget();
-    for (auto &CounterName : CounterNames) {
-      CounterName = CounterName.trim();
-      auto CounterOrError = ET.createCounter(CounterName, State);
-
-      if (!CounterOrError)
-        return CounterOrError.takeError();
-
-      pfm::Counter *Counter = CounterOrError.get().get();
-      if (Reserved == 0) {
-        Reserved = Counter->numValues();
-        CounterValues.reserve(Reserved);
-      } else if (Reserved != Counter->numValues())
-        // It'd be wrong to accumulate vectors of 
diff erent sizes.
-        return make_error<Failure>(
-            llvm::Twine("Inconsistent number of values for counter ")
-                .concat(CounterName)
-                .concat(std::to_string(Counter->numValues()))
-                .concat(" vs expected of ")
-                .concat(std::to_string(Reserved)));
-      Scratch->clear();
-      {
-        auto PS = ET.withSavedState();
-        CrashRecoveryContext CRC;
-        CrashRecoveryContext::Enable();
-        const bool Crashed = !CRC.RunSafely([this, Counter, ScratchPtr]() {
-          Counter->start();
-          this->Function(ScratchPtr);
-          Counter->stop();
-        });
-        CrashRecoveryContext::Disable();
-        PS.reset();
-        if (Crashed) {
-          std::string Msg = "snippet crashed while running";
+    char *const ScratchPtr = Scratch->ptr();
+    auto CounterOrError = ET.createCounter(CounterName, State);
+
+    if (!CounterOrError)
+      return CounterOrError.takeError();
+
+    pfm::Counter *Counter = CounterOrError.get().get();
+    Scratch->clear();
+    {
+      auto PS = ET.withSavedState();
+      CrashRecoveryContext CRC;
+      CrashRecoveryContext::Enable();
+      const bool Crashed = !CRC.RunSafely([this, Counter, ScratchPtr]() {
+        Counter->start();
+        this->Function(ScratchPtr);
+        Counter->stop();
+      });
+      CrashRecoveryContext::Disable();
+      PS.reset();
+      if (Crashed) {
+        std::string Msg = "snippet crashed while running";
 #ifdef LLVM_ON_UNIX
-          // See "Exit Status for Commands":
-          // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
-          constexpr const int kSigOffset = 128;
-          if (const char *const SigName = strsignal(CRC.RetCode - kSigOffset)) {
-            Msg += ": ";
-            Msg += SigName;
-          }
-#endif
-          return make_error<SnippetCrash>(std::move(Msg));
+        // See "Exit Status for Commands":
+        // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
+        constexpr const int kSigOffset = 128;
+        if (const char *const SigName = strsignal(CRC.RetCode - kSigOffset)) {
+          Msg += ": ";
+          Msg += SigName;
         }
+#endif
+        return make_error<SnippetCrash>(std::move(Msg));
       }
-
-      auto ValueOrError = Counter->readOrError(Function.getFunctionBytes());
-      if (!ValueOrError)
-        return ValueOrError.takeError();
-      accumulateCounterValues(ValueOrError.get(), &CounterValues);
     }
-    return CounterValues;
+
+    return Counter->readOrError(Function.getFunctionBytes());
   }
 
   const LLVMState &State;

diff  --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
index aff69fca6ed1..730735a46290 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
@@ -89,8 +89,15 @@ class BenchmarkRunner {
   public:
     virtual ~FunctionExecutor();
 
+    Expected<llvm::SmallVector<int64_t, 4>>
+    runAndSample(const char *Counters) const;
+
+  protected:
+    static void
+    accumulateCounterValues(const llvm::SmallVectorImpl<int64_t> &NewValues,
+                            llvm::SmallVectorImpl<int64_t> *Result);
     virtual Expected<llvm::SmallVector<int64_t, 4>>
-    runAndSample(const char *Counters) const = 0;
+    runWithCounter(StringRef CounterName) const = 0;
   };
 
 protected: