[clang] [libc] [llvm] [clang-tools-extra] [llvm-exegesis] Add middle half repetition mode (PR #77020)

Aiden Grossman via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 26 20:07:21 PST 2024


https://github.com/boomanaiden154 updated https://github.com/llvm/llvm-project/pull/77020

>From c5bac325802c65f65a8d99e8d82a7e8079c21fb4 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Thu, 4 Jan 2024 01:47:49 -0800
Subject: [PATCH 1/4] [llvm-exegesis] Add middle half repetition mode

This patch adds two new repetition modes to llvm-exegesis, particularly
loop and duplicate repetition modes of what I am terming the middle half
repetition mode. The middle half repetition mode essentially runs each
measurement twice, one with twice the number of iterations of the other.
These two measurements are then agregated by taking their difference.
This subtracts away any setup/overhead that is unrelated to the code in
the snippet, providing more accurate results.

Using this mode on a couple toy examples, I am able to get exact
(integer) throughput values on all of them in contrast to the default
duplicate/loop repetition modes which show a little bit of noise on the
snippet value.
---
 .../tools/llvm-exegesis/lib/BenchmarkResult.h | 12 +++-
 .../llvm-exegesis/lib/SnippetRepetitor.cpp    |  2 +
 llvm/tools/llvm-exegesis/llvm-exegesis.cpp    | 67 ++++++++++++++-----
 3 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
index 0d08febae20cb3e..3be58fa7f6853c3 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
@@ -78,7 +78,7 @@ struct BenchmarkKey {
 struct BenchmarkMeasure {
   // A helper to create an unscaled BenchmarkMeasure.
   static BenchmarkMeasure Create(std::string Key, double Value) {
-    return {Key, Value, Value};
+    return {Key, Value, Value, Value};
   }
   std::string Key;
   // This is the per-instruction value, i.e. measured quantity scaled per
@@ -87,6 +87,8 @@ struct BenchmarkMeasure {
   // This is the per-snippet value, i.e. measured quantity for one repetition of
   // the whole snippet.
   double PerSnippetValue;
+  // This is the raw value collected from the full execution.
+  double RawValue;
 };
 
 // The result of an instruction benchmark.
@@ -101,7 +103,13 @@ struct Benchmark {
   // The number of instructions inside the repeated snippet. For example, if a
   // snippet of 3 instructions is repeated 4 times, this is 12.
   unsigned NumRepetitions = 0;
-  enum RepetitionModeE { Duplicate, Loop, AggregateMin };
+  enum RepetitionModeE {
+    Duplicate,
+    Loop,
+    AggregateMin,
+    MiddleHalfDuplicate,
+    MiddleHalfLoop
+  };
   // Note that measurements are per instruction.
   std::vector<BenchmarkMeasure> Measurements;
   std::string Error;
diff --git a/llvm/tools/llvm-exegesis/lib/SnippetRepetitor.cpp b/llvm/tools/llvm-exegesis/lib/SnippetRepetitor.cpp
index cc5a045a8be5dd4..7e19e8a9bb52118 100644
--- a/llvm/tools/llvm-exegesis/lib/SnippetRepetitor.cpp
+++ b/llvm/tools/llvm-exegesis/lib/SnippetRepetitor.cpp
@@ -135,8 +135,10 @@ SnippetRepetitor::Create(Benchmark::RepetitionModeE Mode,
                          const LLVMState &State) {
   switch (Mode) {
   case Benchmark::Duplicate:
+  case Benchmark::MiddleHalfDuplicate:
     return std::make_unique<DuplicateSnippetRepetitor>(State);
   case Benchmark::Loop:
+  case Benchmark::MiddleHalfLoop:
     return std::make_unique<LoopSnippetRepetitor>(State);
   case Benchmark::AggregateMin:
     break;
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index ffbf94ce0fcb267..b963ca0252a72ed 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -106,10 +106,13 @@ static cl::opt<exegesis::Benchmark::RepetitionModeE> RepetitionMode(
     cl::values(
         clEnumValN(exegesis::Benchmark::Duplicate, "duplicate",
                    "Duplicate the snippet"),
-        clEnumValN(exegesis::Benchmark::Loop, "loop",
-                   "Loop over the snippet"),
+        clEnumValN(exegesis::Benchmark::Loop, "loop", "Loop over the snippet"),
         clEnumValN(exegesis::Benchmark::AggregateMin, "min",
-                   "All of the above and take the minimum of measurements")),
+                   "All of the above and take the minimum of measurements"),
+        clEnumValN(exegesis::Benchmark::MiddleHalfDuplicate,
+                   "middle-half-duplicate", "Middle half duplicate mode"),
+        clEnumValN(exegesis::Benchmark::MiddleHalfLoop, "middle-half-loop",
+                   "Middle half loop mode")),
     cl::init(exegesis::Benchmark::Duplicate));
 
 static cl::opt<bool> BenchmarkMeasurementsPrintProgress(
@@ -399,29 +402,37 @@ static void runBenchmarkConfigurations(
   std::optional<ProgressMeter<>> Meter;
   if (BenchmarkMeasurementsPrintProgress)
     Meter.emplace(Configurations.size());
+
+  SmallVector<unsigned, 2> MinInstructions = {NumRepetitions};
+  if (RepetitionMode == Benchmark::MiddleHalfDuplicate ||
+      RepetitionMode == Benchmark::MiddleHalfLoop)
+    MinInstructions.push_back(NumRepetitions * 2);
+
   for (const BenchmarkCode &Conf : Configurations) {
     ProgressMeter<>::ProgressMeterStep MeterStep(Meter ? &*Meter : nullptr);
     SmallVector<Benchmark, 2> AllResults;
 
     for (const std::unique_ptr<const SnippetRepetitor> &Repetitor :
          Repetitors) {
-      auto RC = ExitOnErr(Runner.getRunnableConfiguration(
-          Conf, NumRepetitions, LoopBodySize, *Repetitor));
-      std::optional<StringRef> DumpFile;
-      if (DumpObjectToDisk.getNumOccurrences())
-        DumpFile = DumpObjectToDisk;
-      auto [Err, BenchmarkResult] =
-          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.
-        if (!Err.isA<SnippetExecutionFailure>()) {
-          llvm::errs() << "llvm-exegesis error: " << toString(std::move(Err));
-          exit(1);
+      for (unsigned IterationRepetitions : MinInstructions) {
+        auto RC = ExitOnErr(Runner.getRunnableConfiguration(
+            Conf, IterationRepetitions, LoopBodySize, *Repetitor));
+        std::optional<StringRef> DumpFile;
+        if (DumpObjectToDisk.getNumOccurrences())
+          DumpFile = DumpObjectToDisk;
+        auto [Err, BenchmarkResult] =
+            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.
+          if (!Err.isA<SnippetExecutionFailure>()) {
+            llvm::errs() << "llvm-exegesis error: " << toString(std::move(Err));
+            exit(1);
+          }
+          BenchmarkResult.Error = toString(std::move(Err));
         }
-        BenchmarkResult.Error = toString(std::move(Err));
+        AllResults.push_back(std::move(BenchmarkResult));
       }
-      AllResults.push_back(std::move(BenchmarkResult));
     }
     Benchmark &Result = AllResults.front();
 
@@ -455,6 +466,26 @@ static void runBenchmarkConfigurations(
               Measurement.PerSnippetValue, NewMeasurement.PerSnippetValue);
         }
       }
+    } else if (RepetitionMode ==
+                   Benchmark::RepetitionModeE::MiddleHalfDuplicate ||
+               RepetitionMode == Benchmark::RepetitionModeE::MiddleHalfLoop) {
+      for (const Benchmark &OtherResult :
+           ArrayRef<Benchmark>(AllResults).drop_front()) {
+        if (OtherResult.Measurements.empty())
+          continue;
+        assert(OtherResult.Measurements.size() == Result.Measurements.size());
+        for (auto I : zip(Result.Measurements, OtherResult.Measurements)) {
+          BenchmarkMeasure &Measurement = std::get<0>(I);
+          const BenchmarkMeasure &NewMeasurement = std::get<1>(I);
+          Measurement.RawValue = NewMeasurement.RawValue - Measurement.RawValue;
+          Measurement.PerInstructionValue = Measurement.RawValue;
+          Measurement.PerInstructionValue /= Result.NumRepetitions;
+          Measurement.PerSnippetValue = Measurement.RawValue;
+          Measurement.PerSnippetValue *=
+              static_cast<double>(Result.Key.Instructions.size()) /
+              Result.NumRepetitions;
+        }
+      }
     }
 
     // With dummy counters, measurements are rather meaningless,

>From 8c3640bc094859d711fd8a32bad9ea978e9db443 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 26 Jan 2024 18:56:55 -0800
Subject: [PATCH 2/4] Refactor into a separate file

---
 llvm/tools/llvm-exegesis/lib/CMakeLists.txt   |   1 +
 .../llvm-exegesis/lib/ResultAggregator.cpp    | 105 ++++++++++++++++++
 .../llvm-exegesis/lib/ResultAggregator.h      |  35 ++++++
 llvm/tools/llvm-exegesis/llvm-exegesis.cpp    |  50 +--------
 4 files changed, 147 insertions(+), 44 deletions(-)
 create mode 100644 llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp
 create mode 100644 llvm/tools/llvm-exegesis/lib/ResultAggregator.h

diff --git a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
index 7312811989dd800..6ae441d31f07fe8 100644
--- a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
+++ b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
@@ -64,6 +64,7 @@ add_llvm_library(LLVMExegesis
   PerfHelper.cpp
   RegisterAliasing.cpp
   RegisterValue.cpp
+  ResultAggregator.cpp
   SchedClassResolution.cpp
   SerialSnippetGenerator.cpp
   SnippetFile.cpp
diff --git a/llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp b/llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp
new file mode 100644
index 000000000000000..08cd735167991d2
--- /dev/null
+++ b/llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp
@@ -0,0 +1,105 @@
+//===-- ResultAggregator.cpp ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Implements result aggregators that are used to aggregate the results from
+/// multiple full benchmark runs.
+///
+//===----------------------------------------------------------------------===//
+
+#include "ResultAggregator.h"
+
+namespace llvm {
+namespace exegesis {
+
+class DefaultResultAggregator : public ResultAggregator {
+  void AggregateResults(Benchmark &Result,
+                        ArrayRef<Benchmark> OtherResults) const override;
+};
+
+// We don't need to do anything in the default result aggregator as the result
+// we want is the first one, which is passed in here as Result.
+void DefaultResultAggregator::AggregateResults(
+    Benchmark &Result, ArrayRef<Benchmark> OtherResults) const {}
+
+class MinimumResultAggregator : public ResultAggregator {
+  void AggregateMeasurement(BenchmarkMeasure &Measurement,
+                            const BenchmarkMeasure &NewMeasurement,
+                            const Benchmark &Result) const override;
+};
+
+void MinimumResultAggregator::AggregateMeasurement(
+    BenchmarkMeasure &Measurement, const BenchmarkMeasure &NewMeasurement,
+    const Benchmark &Result) const {
+  Measurement.PerInstructionValue = std::min(
+      Measurement.PerInstructionValue, NewMeasurement.PerInstructionValue);
+  Measurement.PerSnippetValue =
+      std::min(Measurement.PerSnippetValue, NewMeasurement.PerSnippetValue);
+}
+
+class MiddleHalfResultAggregator : public ResultAggregator {
+  void AggregateMeasurement(BenchmarkMeasure &Measurement,
+                            const BenchmarkMeasure &NewMeasurement,
+                            const Benchmark &Result) const override;
+};
+
+void MiddleHalfResultAggregator::AggregateMeasurement(
+    BenchmarkMeasure &Measurement, const BenchmarkMeasure &NewMeasurement,
+    const Benchmark &Result) const {
+  Measurement.RawValue = NewMeasurement.RawValue - Measurement.RawValue;
+  Measurement.PerInstructionValue = Measurement.RawValue;
+  Measurement.PerInstructionValue /= Result.NumRepetitions;
+  Measurement.PerSnippetValue = Measurement.RawValue;
+  Measurement.PerSnippetValue /=
+      std::ceil(Result.NumRepetitions /
+                static_cast<double>(Result.Key.Instructions.size()));
+}
+
+void ResultAggregator::AggregateMeasurement(
+    BenchmarkMeasure &Measurement, const BenchmarkMeasure &NewMeasurement,
+    const Benchmark &Result) const {}
+
+void ResultAggregator::AggregateResults(
+    Benchmark &Result, ArrayRef<Benchmark> OtherResults) const {
+  for (const Benchmark &OtherResult : OtherResults) {
+    append_range(Result.AssembledSnippet, OtherResult.AssembledSnippet);
+
+    if (OtherResult.Measurements.empty())
+      continue;
+
+    assert(OtherResult.Measurements.size() == Result.Measurements.size() &&
+           "Expected to have an identical number of measurements");
+
+    for (auto I : zip(Result.Measurements, OtherResult.Measurements)) {
+      BenchmarkMeasure &Measurement = std::get<0>(I);
+      const BenchmarkMeasure &NewMeasurement = std::get<1>(I);
+
+      assert(Measurement.Key == NewMeasurement.Key &&
+             "Expected measurements to be symmetric");
+
+      AggregateMeasurement(Measurement, NewMeasurement, Result);
+    }
+  }
+}
+
+ResultAggregator
+ResultAggregator::CreateAggregator(Benchmark::RepetitionModeE RepetitionMode) {
+  switch (RepetitionMode) {
+  case Benchmark::RepetitionModeE::Duplicate:
+  case Benchmark::RepetitionModeE::Loop:
+    return DefaultResultAggregator();
+  case Benchmark::RepetitionModeE::AggregateMin:
+    return MinimumResultAggregator();
+  case Benchmark::RepetitionModeE::MiddleHalfDuplicate:
+  case Benchmark::RepetitionModeE::MiddleHalfLoop:
+    return MiddleHalfResultAggregator();
+  }
+}
+
+} // namespace exegesis
+} // namespace llvm
diff --git a/llvm/tools/llvm-exegesis/lib/ResultAggregator.h b/llvm/tools/llvm-exegesis/lib/ResultAggregator.h
new file mode 100644
index 000000000000000..2f148baf7eef52e
--- /dev/null
+++ b/llvm/tools/llvm-exegesis/lib/ResultAggregator.h
@@ -0,0 +1,35 @@
+//===-- ResultAggregator.h --------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Defines result aggregators that are used to aggregate the results from
+/// multiple full benchmark runs.
+///
+//===----------------------------------------------------------------------===//
+
+#include "BenchmarkResult.h"
+
+namespace llvm {
+namespace exegesis {
+
+class ResultAggregator {
+public:
+  static ResultAggregator
+  CreateAggregator(Benchmark::RepetitionModeE RepetitionMode);
+
+  virtual void AggregateResults(Benchmark &Result,
+                                ArrayRef<Benchmark> OtherResults) const;
+  virtual void AggregateMeasurement(BenchmarkMeasure &Measurement,
+                                    const BenchmarkMeasure &NewMeasurement,
+                                    const Benchmark &Result) const;
+
+  virtual ~ResultAggregator() = default;
+};
+
+} // namespace exegesis
+} // namespace llvm
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index e94fcef9ee5813e..c4816a955ffdf41 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -20,6 +20,7 @@
 #include "lib/LlvmState.h"
 #include "lib/PerfHelper.h"
 #include "lib/ProgressMeter.h"
+#include "lib/ResultAggregator.h"
 #include "lib/SnippetFile.h"
 #include "lib/SnippetRepetitor.h"
 #include "lib/Target.h"
@@ -456,6 +457,7 @@ static void runBenchmarkConfigurations(
         AllResults.push_back(std::move(BenchmarkResult));
       }
     }
+
     Benchmark &Result = AllResults.front();
 
     // If any of our measurements failed, pretend they all have failed.
@@ -465,50 +467,10 @@ static void runBenchmarkConfigurations(
         }))
       Result.Measurements.clear();
 
-    if (RepetitionMode == Benchmark::RepetitionModeE::AggregateMin) {
-      for (const Benchmark &OtherResult :
-           ArrayRef<Benchmark>(AllResults).drop_front()) {
-        llvm::append_range(Result.AssembledSnippet,
-                           OtherResult.AssembledSnippet);
-        // Aggregate measurements, but only if all measurements succeeded.
-        if (Result.Measurements.empty())
-          continue;
-        assert(OtherResult.Measurements.size() == Result.Measurements.size() &&
-               "Expected to have identical number of measurements.");
-        for (auto I : zip(Result.Measurements, OtherResult.Measurements)) {
-          BenchmarkMeasure &Measurement = std::get<0>(I);
-          const BenchmarkMeasure &NewMeasurement = std::get<1>(I);
-          assert(Measurement.Key == NewMeasurement.Key &&
-                 "Expected measurements to be symmetric");
-
-          Measurement.PerInstructionValue =
-              std::min(Measurement.PerInstructionValue,
-                       NewMeasurement.PerInstructionValue);
-          Measurement.PerSnippetValue = std::min(
-              Measurement.PerSnippetValue, NewMeasurement.PerSnippetValue);
-        }
-      }
-    } else if (RepetitionMode ==
-                   Benchmark::RepetitionModeE::MiddleHalfDuplicate ||
-               RepetitionMode == Benchmark::RepetitionModeE::MiddleHalfLoop) {
-      for (const Benchmark &OtherResult :
-           ArrayRef<Benchmark>(AllResults).drop_front()) {
-        if (OtherResult.Measurements.empty())
-          continue;
-        assert(OtherResult.Measurements.size() == Result.Measurements.size());
-        for (auto I : zip(Result.Measurements, OtherResult.Measurements)) {
-          BenchmarkMeasure &Measurement = std::get<0>(I);
-          const BenchmarkMeasure &NewMeasurement = std::get<1>(I);
-          Measurement.RawValue = NewMeasurement.RawValue - Measurement.RawValue;
-          Measurement.PerInstructionValue = Measurement.RawValue;
-          Measurement.PerInstructionValue /= Result.NumRepetitions;
-          Measurement.PerSnippetValue = Measurement.RawValue;
-          Measurement.PerSnippetValue *=
-              static_cast<double>(Result.Key.Instructions.size()) /
-              Result.NumRepetitions;
-        }
-      }
-    }
+    ResultAggregator ResultAgg =
+        ResultAggregator::CreateAggregator(RepetitionMode);
+    ResultAgg.AggregateResults(Result,
+                               ArrayRef<Benchmark>(AllResults).drop_front());
 
     // With dummy counters, measurements are rather meaningless,
     // so drop them altogether.

>From 3eec8129676dec80bebd0c29b10d7725498f2f4a Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 26 Jan 2024 19:55:52 -0800
Subject: [PATCH 3/4] Add unit tests

---
 .../llvm-exegesis/lib/ResultAggregator.cpp    | 30 +++----
 .../llvm-exegesis/lib/ResultAggregator.h      |  4 +-
 llvm/tools/llvm-exegesis/llvm-exegesis.cpp    |  6 +-
 .../tools/llvm-exegesis/CMakeLists.txt        |  1 +
 .../llvm-exegesis/ResultAggregatorTest.cpp    | 78 +++++++++++++++++++
 5 files changed, 94 insertions(+), 25 deletions(-)
 create mode 100644 llvm/unittests/tools/llvm-exegesis/ResultAggregatorTest.cpp

diff --git a/llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp b/llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp
index 08cd735167991d2..49403aed90ab716 100644
--- a/llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp
+++ b/llvm/tools/llvm-exegesis/lib/ResultAggregator.cpp
@@ -5,12 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-///
-/// \file
-/// Implements result aggregators that are used to aggregate the results from
-/// multiple full benchmark runs.
-///
-//===----------------------------------------------------------------------===//
 
 #include "ResultAggregator.h"
 
@@ -19,14 +13,12 @@ namespace exegesis {
 
 class DefaultResultAggregator : public ResultAggregator {
   void AggregateResults(Benchmark &Result,
-                        ArrayRef<Benchmark> OtherResults) const override;
+                        ArrayRef<Benchmark> OtherResults) const override{};
+  void AggregateMeasurement(BenchmarkMeasure &Measurement,
+                            const BenchmarkMeasure &NewMeasurement,
+                            const Benchmark &Result) const override{};
 };
 
-// We don't need to do anything in the default result aggregator as the result
-// we want is the first one, which is passed in here as Result.
-void DefaultResultAggregator::AggregateResults(
-    Benchmark &Result, ArrayRef<Benchmark> OtherResults) const {}
-
 class MinimumResultAggregator : public ResultAggregator {
   void AggregateMeasurement(BenchmarkMeasure &Measurement,
                             const BenchmarkMeasure &NewMeasurement,
@@ -40,6 +32,8 @@ void MinimumResultAggregator::AggregateMeasurement(
       Measurement.PerInstructionValue, NewMeasurement.PerInstructionValue);
   Measurement.PerSnippetValue =
       std::min(Measurement.PerSnippetValue, NewMeasurement.PerSnippetValue);
+  Measurement.RawValue =
+      std::min(Measurement.RawValue, NewMeasurement.RawValue);
 }
 
 class MiddleHalfResultAggregator : public ResultAggregator {
@@ -60,10 +54,6 @@ void MiddleHalfResultAggregator::AggregateMeasurement(
                 static_cast<double>(Result.Key.Instructions.size()));
 }
 
-void ResultAggregator::AggregateMeasurement(
-    BenchmarkMeasure &Measurement, const BenchmarkMeasure &NewMeasurement,
-    const Benchmark &Result) const {}
-
 void ResultAggregator::AggregateResults(
     Benchmark &Result, ArrayRef<Benchmark> OtherResults) const {
   for (const Benchmark &OtherResult : OtherResults) {
@@ -87,17 +77,17 @@ void ResultAggregator::AggregateResults(
   }
 }
 
-ResultAggregator
+std::unique_ptr<ResultAggregator>
 ResultAggregator::CreateAggregator(Benchmark::RepetitionModeE RepetitionMode) {
   switch (RepetitionMode) {
   case Benchmark::RepetitionModeE::Duplicate:
   case Benchmark::RepetitionModeE::Loop:
-    return DefaultResultAggregator();
+    return std::make_unique<DefaultResultAggregator>();
   case Benchmark::RepetitionModeE::AggregateMin:
-    return MinimumResultAggregator();
+    return std::make_unique<MinimumResultAggregator>();
   case Benchmark::RepetitionModeE::MiddleHalfDuplicate:
   case Benchmark::RepetitionModeE::MiddleHalfLoop:
-    return MiddleHalfResultAggregator();
+    return std::make_unique<MiddleHalfResultAggregator>();
   }
 }
 
diff --git a/llvm/tools/llvm-exegesis/lib/ResultAggregator.h b/llvm/tools/llvm-exegesis/lib/ResultAggregator.h
index 2f148baf7eef52e..c154ea98d046ce1 100644
--- a/llvm/tools/llvm-exegesis/lib/ResultAggregator.h
+++ b/llvm/tools/llvm-exegesis/lib/ResultAggregator.h
@@ -19,14 +19,14 @@ namespace exegesis {
 
 class ResultAggregator {
 public:
-  static ResultAggregator
+  static std::unique_ptr<ResultAggregator>
   CreateAggregator(Benchmark::RepetitionModeE RepetitionMode);
 
   virtual void AggregateResults(Benchmark &Result,
                                 ArrayRef<Benchmark> OtherResults) const;
   virtual void AggregateMeasurement(BenchmarkMeasure &Measurement,
                                     const BenchmarkMeasure &NewMeasurement,
-                                    const Benchmark &Result) const;
+                                    const Benchmark &Result) const = 0;
 
   virtual ~ResultAggregator() = default;
 };
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index c4816a955ffdf41..a21fe5dfa86623c 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -467,10 +467,10 @@ static void runBenchmarkConfigurations(
         }))
       Result.Measurements.clear();
 
-    ResultAggregator ResultAgg =
+    std::unique_ptr<ResultAggregator> ResultAgg =
         ResultAggregator::CreateAggregator(RepetitionMode);
-    ResultAgg.AggregateResults(Result,
-                               ArrayRef<Benchmark>(AllResults).drop_front());
+    ResultAgg->AggregateResults(Result,
+                                ArrayRef<Benchmark>(AllResults).drop_front());
 
     // With dummy counters, measurements are rather meaningless,
     // so drop them altogether.
diff --git a/llvm/unittests/tools/llvm-exegesis/CMakeLists.txt b/llvm/unittests/tools/llvm-exegesis/CMakeLists.txt
index fa81c332cd8c9c8..8b2f6334dda8636 100644
--- a/llvm/unittests/tools/llvm-exegesis/CMakeLists.txt
+++ b/llvm/unittests/tools/llvm-exegesis/CMakeLists.txt
@@ -17,6 +17,7 @@ set(exegesis_sources
   ClusteringTest.cpp
   ProgressMeterTest.cpp
   RegisterValueTest.cpp
+  ResultAggregatorTest.cpp
   )
 
 set(exegesis_link_libraries LLVMExegesis)
diff --git a/llvm/unittests/tools/llvm-exegesis/ResultAggregatorTest.cpp b/llvm/unittests/tools/llvm-exegesis/ResultAggregatorTest.cpp
new file mode 100644
index 000000000000000..f316baf1eb220bf
--- /dev/null
+++ b/llvm/unittests/tools/llvm-exegesis/ResultAggregatorTest.cpp
@@ -0,0 +1,78 @@
+//===-- ResultAggregatorTest.cpp --------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ResultAggregator.h"
+#include "gtest/gtest.h"
+
+namespace llvm {
+namespace exegesis {
+
+namespace {
+
+TEST(ResultAggregatorTest, DefaultAggregator) {
+  std::vector<Benchmark> Results(1);
+
+  Results[0].Measurements = {BenchmarkMeasure::Create("x", 1, {})};
+
+  Benchmark &Result = Results[0];
+
+  std::unique_ptr<ResultAggregator> AggregatorToTest =
+      ResultAggregator::CreateAggregator(Benchmark::RepetitionModeE::Duplicate);
+  AggregatorToTest->AggregateResults(Result, ArrayRef(Results).drop_front());
+
+  EXPECT_EQ(Result.Measurements[0].PerInstructionValue, 1);
+  EXPECT_EQ(Result.Measurements[0].PerSnippetValue, 1);
+  EXPECT_EQ(Result.Measurements[0].RawValue, 1);
+}
+
+TEST(ResultAggregatorTest, MinimumResultAggregator) {
+  std::vector<Benchmark> Results(2);
+
+  Results[0].Measurements = {BenchmarkMeasure::Create("x", 2, {})};
+  Results[1].Measurements = {BenchmarkMeasure::Create("x", 1, {})};
+
+  Benchmark &Result = Results[0];
+
+  std::unique_ptr<ResultAggregator> AggregatorToTest =
+      ResultAggregator::CreateAggregator(
+          Benchmark::RepetitionModeE::AggregateMin);
+  AggregatorToTest->AggregateResults(Result, ArrayRef(Results).drop_front());
+
+  EXPECT_EQ(Result.Measurements[0].PerInstructionValue, 1);
+  EXPECT_EQ(Result.Measurements[0].PerSnippetValue, 1);
+  EXPECT_EQ(Result.Measurements[0].RawValue, 1);
+}
+
+TEST(ResultAggregatorTest, MiddleHalfAggregator) {
+  std::vector<Benchmark> Results(2);
+
+  Results[0].Measurements = {BenchmarkMeasure::Create("x", 2, {})};
+  Results[1].Measurements = {BenchmarkMeasure::Create("x", 6, {})};
+
+  Results[0].Key.Instructions.push_back(MCInst());
+  Results[1].Key.Instructions.push_back(MCInst());
+
+  Results[0].NumRepetitions = 1;
+  Results[1].NumRepetitions = 3;
+
+  Benchmark &Result = Results[0];
+
+  std::unique_ptr<ResultAggregator> AggregatorToTest =
+      ResultAggregator::CreateAggregator(
+          Benchmark::RepetitionModeE::MiddleHalfLoop);
+  AggregatorToTest->AggregateResults(Result, ArrayRef(Results).drop_front());
+
+  EXPECT_EQ(Result.Measurements[0].PerInstructionValue, 4);
+  EXPECT_EQ(Result.Measurements[0].PerSnippetValue, 4);
+  EXPECT_EQ(Result.Measurements[0].RawValue, 4);
+}
+
+} // namespace
+
+} // namespace exegesis
+} // namespace llvm

>From f585124f3959cdf24ff05e96379c731db39cfb39 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 26 Jan 2024 20:07:08 -0800
Subject: [PATCH 4/4] Add lit test

---
 llvm/test/tools/llvm-exegesis/X86/latency/middle-half.s | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 llvm/test/tools/llvm-exegesis/X86/latency/middle-half.s

diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/middle-half.s b/llvm/test/tools/llvm-exegesis/X86/latency/middle-half.s
new file mode 100644
index 000000000000000..0c5d51e4bcc5882
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/X86/latency/middle-half.s
@@ -0,0 +1,6 @@
+# REQUIRES: exegesis-can-measure-latency, x86_64-linux
+
+# Check that we can use the middle-half repetition mode without crashing
+
+# RUN: llvm-exegesis -mtriple=x86_64-unknown-unknown -mode=latency -opcode-name=ADD64rr -repetition-mode=middle-half-duplicate
+# RUN: llvm-exegesis -mtriple=x86_64-unknown-unknown -mode=latency -opcode-name=ADD64rr -repetition-mode=middle-half-loop



More information about the cfe-commits mailing list