From 7e75136ee718c81a5095a287ee0d78db9cf25478 Mon Sep 17 00:00:00 2001 From: Min Hsu 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 d357c2ceea418..8266d891a5e6b 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= 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 e4449d7df3d82..0000000000000 --- 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 5480d85616878..3c09a8380146e 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 cc46f7feb6cf7..a7771b99e97b1 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> create(const LLVMState &State, object::OwningBinary Obj, BenchmarkRunner::ScratchSpace *Scratch, - std::optional BenchmarkProcessCPU, bool DryRun) { + std::optional BenchmarkProcessCPU) { Expected EF = ExecutableFunction::create(State.createTargetMachine(), std::move(Obj)); @@ -107,17 +107,14 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor { return EF.takeError(); return std::unique_ptr( - 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 &NewValues, SmallVector *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> BenchmarkRunner::createFunctionExecutor( object::OwningBinary ObjectFile, const BenchmarkKey &Key, std::optional 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("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 e2251ff978888..29e58692f0e92 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( "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 07bd44ee64f1f..fa37e05956be8 100644 --- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp +++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp @@ -132,10 +132,7 @@ static cl::opt 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 @@ -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 &&