[llvm] Revert "[Exegesis] Add the ability to dry-run the measurement phase (… (PR #122371)

Min-Yih Hsu via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 9 12:59:01 PST 2025


https://github.com/mshockwave created https://github.com/llvm/llvm-project/pull/122371

…#121991)"

This reverts commit f8f8598fd886cddfd374fa43eb6d7d37d301b576.

This breaks ARMv7 and s390x buildbot with the following message:
```
llvm-exegesis error: No available targets are compatible with triple "armv8l-unknown-linux-gnueabihf"
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /home/tcwg-buildbot/worker/clang-armv7-2stage/stage2/bin/FileCheck /home/tcwg-buildbot/worker/clang-armv7-2stage/llvm/llvm/test/tools/llvm-exegesis/dry-run-measurement.test
```

Which I'm a little confused because I'm sure it's coming from `TargetSelect::lookupTarget` but...why isn't its own triple got registered?

>From 7e75136ee718c81a5095a287ee0d78db9cf25478 Mon Sep 17 00:00:00 2001
From: Min Hsu <min.hsu at sifive.com>
Date: Thu, 9 Jan 2025 12:55:17 -0800
Subject: [PATCH] Revert "[Exegesis] Add the ability to dry-run the measurement
 phase (#121991)"

This reverts commit f8f8598fd886cddfd374fa43eb6d7d37d301b576.
---
 llvm/docs/CommandGuide/llvm-exegesis.rst      |  1 -
 .../llvm-exegesis/dry-run-measurement.test    | 11 -------
 .../tools/llvm-exegesis/lib/BenchmarkResult.h |  1 -
 .../llvm-exegesis/lib/BenchmarkRunner.cpp     | 33 +++++--------------
 llvm/tools/llvm-exegesis/lib/Target.cpp       |  4 +--
 llvm/tools/llvm-exegesis/llvm-exegesis.cpp    |  9 ++---
 6 files changed, 13 insertions(+), 46 deletions(-)
 delete mode 100644 llvm/test/tools/llvm-exegesis/dry-run-measurement.test

diff --git a/llvm/docs/CommandGuide/llvm-exegesis.rst b/llvm/docs/CommandGuide/llvm-exegesis.rst
index d357c2ceea4189..8266d891a5e6b1 100644
--- a/llvm/docs/CommandGuide/llvm-exegesis.rst
+++ b/llvm/docs/CommandGuide/llvm-exegesis.rst
@@ -301,7 +301,6 @@ OPTIONS
   * ``prepare-and-assemble-snippet``: Same as ``prepare-snippet``, but also dumps an excerpt of the sequence (hex encoded).
   * ``assemble-measured-code``: Same as ``prepare-and-assemble-snippet``. but also creates the full sequence that can be dumped to a file using ``--dump-object-to-disk``.
   * ``measure``: Same as ``assemble-measured-code``, but also runs the measurement.
-  * ``dry-run-measurement``: Same as measure, but does not actually execute the snippet.
 
 .. option:: --x86-lbr-sample-period=<nBranches/sample>
 
diff --git a/llvm/test/tools/llvm-exegesis/dry-run-measurement.test b/llvm/test/tools/llvm-exegesis/dry-run-measurement.test
deleted file mode 100644
index e4449d7df3d826..00000000000000
--- a/llvm/test/tools/llvm-exegesis/dry-run-measurement.test
+++ /dev/null
@@ -1,11 +0,0 @@
-# RUN: llvm-exegesis --mtriple=riscv64 --mcpu=sifive-p470 --mode=latency --opcode-name=ADD --use-dummy-perf-counters --benchmark-phase=dry-run-measurement | FileCheck %s
-# REQUIRES: riscv-registered-target
-
-# This test makes sure that llvm-exegesis doesn't execute "cross-compiled" snippets in the presence of
-# --dry-run-measurement. RISC-V was chosen simply because most of the time we run tests on X86 machines.
-
-# Should not contain misleading results.
-# CHECK: measurements:    []
-
-# Should not contain error messages like "snippet crashed while running: Segmentation fault".
-# CHECK: error:           ''
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
index 5480d856168784..3c09a8380146e5 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
@@ -38,7 +38,6 @@ enum class BenchmarkPhaseSelectorE {
   PrepareAndAssembleSnippet,
   AssembleMeasuredCode,
   Measure,
-  DryRunMeasure,
 };
 
 enum class BenchmarkFilter { All, RegOnly, WithMem };
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index cc46f7feb6cf7f..a7771b99e97b1a 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -99,7 +99,7 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
   static Expected<std::unique_ptr<InProcessFunctionExecutorImpl>>
   create(const LLVMState &State, object::OwningBinary<object::ObjectFile> Obj,
          BenchmarkRunner::ScratchSpace *Scratch,
-         std::optional<int> BenchmarkProcessCPU, bool DryRun) {
+         std::optional<int> BenchmarkProcessCPU) {
     Expected<ExecutableFunction> EF =
         ExecutableFunction::create(State.createTargetMachine(), std::move(Obj));
 
@@ -107,17 +107,14 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
       return EF.takeError();
 
     return std::unique_ptr<InProcessFunctionExecutorImpl>(
-        new InProcessFunctionExecutorImpl(State, std::move(*EF), Scratch,
-                                          DryRun));
+        new InProcessFunctionExecutorImpl(State, std::move(*EF), Scratch));
   }
 
 private:
   InProcessFunctionExecutorImpl(const LLVMState &State,
                                 ExecutableFunction Function,
-                                BenchmarkRunner::ScratchSpace *Scratch,
-                                bool DryRun)
-      : State(State), Function(std::move(Function)), Scratch(Scratch),
-        DryRun(DryRun) {}
+                                BenchmarkRunner::ScratchSpace *Scratch)
+      : State(State), Function(std::move(Function)), Scratch(Scratch) {}
 
   static void accumulateCounterValues(const SmallVector<int64_t, 4> &NewValues,
                                       SmallVector<int64_t, 4> *Result) {
@@ -146,14 +143,9 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
       CrashRecoveryContext CRC;
       CrashRecoveryContext::Enable();
       const bool Crashed = !CRC.RunSafely([this, Counter, ScratchPtr]() {
-        if (DryRun) {
-          Counter->start();
-          Counter->stop();
-        } else {
-          Counter->start();
-          this->Function(ScratchPtr);
-          Counter->stop();
-        }
+        Counter->start();
+        this->Function(ScratchPtr);
+        Counter->stop();
       });
       CrashRecoveryContext::Disable();
       PS.reset();
@@ -185,7 +177,6 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
   const LLVMState &State;
   const ExecutableFunction Function;
   BenchmarkRunner::ScratchSpace *const Scratch;
-  bool DryRun = false;
 };
 
 #ifdef __linux__
@@ -673,9 +664,6 @@ Expected<std::unique_ptr<BenchmarkRunner::FunctionExecutor>>
 BenchmarkRunner::createFunctionExecutor(
     object::OwningBinary<object::ObjectFile> ObjectFile,
     const BenchmarkKey &Key, std::optional<int> BenchmarkProcessCPU) const {
-  bool DryRun =
-      BenchmarkPhaseSelector == BenchmarkPhaseSelectorE::DryRunMeasure;
-
   switch (ExecutionMode) {
   case ExecutionModeE::InProcess: {
     if (BenchmarkProcessCPU.has_value())
@@ -683,8 +671,7 @@ BenchmarkRunner::createFunctionExecutor(
                                  "support benchmark core pinning.");
 
     auto InProcessExecutorOrErr = InProcessFunctionExecutorImpl::create(
-        State, std::move(ObjectFile), Scratch.get(), BenchmarkProcessCPU,
-        DryRun);
+        State, std::move(ObjectFile), Scratch.get(), BenchmarkProcessCPU);
     if (!InProcessExecutorOrErr)
       return InProcessExecutorOrErr.takeError();
 
@@ -692,10 +679,6 @@ BenchmarkRunner::createFunctionExecutor(
   }
   case ExecutionModeE::SubProcess: {
 #ifdef __linux__
-    if (DryRun)
-      return make_error<Failure>("The subprocess execution mode cannot "
-                                 "dry-run measurement at this moment.");
-
     auto SubProcessExecutorOrErr = SubProcessFunctionExecutorImpl::create(
         State, std::move(ObjectFile), Key, BenchmarkProcessCPU);
     if (!SubProcessExecutorOrErr)
diff --git a/llvm/tools/llvm-exegesis/lib/Target.cpp b/llvm/tools/llvm-exegesis/lib/Target.cpp
index e2251ff978888b..29e58692f0e92b 100644
--- a/llvm/tools/llvm-exegesis/lib/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/Target.cpp
@@ -98,7 +98,7 @@ ExegesisTarget::createBenchmarkRunner(
     return nullptr;
   case Benchmark::Latency:
   case Benchmark::InverseThroughput:
-    if (BenchmarkPhaseSelector >= BenchmarkPhaseSelectorE::Measure &&
+    if (BenchmarkPhaseSelector == BenchmarkPhaseSelectorE::Measure &&
         !PfmCounters.CycleCounter) {
       const char *ModeName = Mode == Benchmark::Latency
                                  ? "latency"
@@ -116,7 +116,7 @@ ExegesisTarget::createBenchmarkRunner(
         State, Mode, BenchmarkPhaseSelector, ResultAggMode, ExecutionMode,
         ValidationCounters, BenchmarkRepeatCount);
   case Benchmark::Uops:
-    if (BenchmarkPhaseSelector >= BenchmarkPhaseSelectorE::Measure &&
+    if (BenchmarkPhaseSelector == BenchmarkPhaseSelectorE::Measure &&
         !PfmCounters.UopsCounter && !PfmCounters.IssueCounters)
       return make_error<Failure>(
           "can't run 'uops' mode, sched model does not define uops or issue "
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index 07bd44ee64f1f2..fa37e05956be8c 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -132,10 +132,7 @@ static cl::opt<BenchmarkPhaseSelectorE> BenchmarkPhaseSelector(
         clEnumValN(
             BenchmarkPhaseSelectorE::Measure, "measure",
             "Same as prepare-measured-code, but also runs the measurement "
-            "(default)"),
-        clEnumValN(
-            BenchmarkPhaseSelectorE::DryRunMeasure, "dry-run-measurement",
-            "Same as measure, but does not actually execute the snippet")),
+            "(default)")),
     cl::init(BenchmarkPhaseSelectorE::Measure));
 
 static cl::opt<bool>
@@ -479,7 +476,7 @@ static void runBenchmarkConfigurations(
 }
 
 void benchmarkMain() {
-  if (BenchmarkPhaseSelector >= BenchmarkPhaseSelectorE::Measure &&
+  if (BenchmarkPhaseSelector == BenchmarkPhaseSelectorE::Measure &&
       !UseDummyPerfCounters) {
 #ifndef HAVE_LIBPFM
     ExitWithError(
@@ -504,7 +501,7 @@ void benchmarkMain() {
 
   // Preliminary check to ensure features needed for requested
   // benchmark mode are present on target CPU and/or OS.
-  if (BenchmarkPhaseSelector >= BenchmarkPhaseSelectorE::Measure)
+  if (BenchmarkPhaseSelector == BenchmarkPhaseSelectorE::Measure)
     ExitOnErr(State.getExegesisTarget().checkFeatureSupport());
 
   if (ExecutionMode == BenchmarkRunner::ExecutionModeE::SubProcess &&



More information about the llvm-commits mailing list