[llvm-exegesis] Refactor common parts out of FunctionExecutorImpl

[aidengrossman]

Summary

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.

[gchatelet]

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h

96–97

The parameters should be named

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h

public:
   };
 
   // A helper to measure counters while executing a function in a sandboxed
   // context.
   class FunctionExecutor {
   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:
   const LLVMState &State;
   const Benchmark::ModeE Mode;
   const BenchmarkPhaseSelectorE BenchmarkPhaseSelector;
 
 private:

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

 BenchmarkRunner::BenchmarkRunner(const LLVMState &State,
                                  Benchmark::ModeE Mode,
                                  BenchmarkPhaseSelectorE BenchmarkPhaseSelector)
     : State(State), Mode(Mode), BenchmarkPhaseSelector(BenchmarkPhaseSelector),
       Scratch(std::make_unique<ScratchSpace>()) {}
 
 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:
   FunctionExecutorImpl(const LLVMState &State,
                        object::OwningBinary<object::ObjectFile> Obj,
                        BenchmarkRunner::ScratchSpace *Scratch)
       : State(State), Function(State.createTargetMachine(), std::move(Obj)),
         Scratch(Scratch) {}
 
 private:
   static void
   accumulateCounterValues(const llvm::SmallVector<int64_t, 4> &NewValues,
                           llvm::SmallVector<int64_t, 4> *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>>
-  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);
+    char *const ScratchPtr = Scratch->ptr();
 
-      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 different 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";
 #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));
-        }
-      }
 
-      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;
   const ExecutableFunction Function;
   BenchmarkRunner::ScratchSpace *const Scratch;
 };
 } // namespace