[llvm] 17e2024 - [NFCI][llvm-exegesis] Extract 'Min' repetition handling from `BenchmarkRunner` into it's caller
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 17 12:15:10 PST 2022
Author: Roman Lebedev
Date: 2022-12-17T23:14:52+03:00
New Revision: 17e202424c021fd903950fec7a8b6cca2d83abce
URL: https://github.com/llvm/llvm-project/commit/17e202424c021fd903950fec7a8b6cca2d83abce
DIFF: https://github.com/llvm/llvm-project/commit/17e202424c021fd903950fec7a8b6cca2d83abce.diff
LOG: [NFCI][llvm-exegesis] Extract 'Min' repetition handling from `BenchmarkRunner` into it's caller
If `BenchmarkRunner::runConfiguration()` deals with more than a single
repetitor, tasking will be less straight-forward to implement.
But i think dealing with that in it's callee is even more readable.
Added:
Modified:
llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
llvm/tools/llvm-exegesis/llvm-exegesis.cpp
Removed:
################################################################################
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index 70e3911b2945..f8da149d30d7 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -137,8 +137,7 @@ class FunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
Expected<InstructionBenchmark> BenchmarkRunner::runConfiguration(
const BenchmarkCode &BC, unsigned NumRepetitions, unsigned LoopBodySize,
- ArrayRef<std::unique_ptr<const SnippetRepetitor>> Repetitors,
- bool DumpObjectToDisk) const {
+ const SnippetRepetitor &Repetitor, bool DumpObjectToDisk) const {
InstructionBenchmark InstrBenchmark;
InstrBenchmark.Mode = Mode;
InstrBenchmark.CpuName = std::string(State.getTargetMachine().getTargetCPU());
@@ -151,23 +150,7 @@ Expected<InstructionBenchmark> BenchmarkRunner::runConfiguration(
InstrBenchmark.Key = BC.Key;
- // If we end up having an error, and we've previously succeeded with
- // some other Repetitor, we want to discard the previous measurements.
- struct ClearBenchmarkOnReturn {
- ClearBenchmarkOnReturn(InstructionBenchmark *IB) : IB(IB) {}
- ~ClearBenchmarkOnReturn() {
- if (Clear)
- IB->Measurements.clear();
- }
- void disarm() { Clear = false; }
-
- private:
- InstructionBenchmark *const IB;
- bool Clear = true;
- };
- ClearBenchmarkOnReturn CBOR(&InstrBenchmark);
-
- for (const std::unique_ptr<const SnippetRepetitor> &Repetitor : Repetitors) {
+ {
// Assemble at least kMinInstructionsForSnippet instructions by repeating
// the snippet for debug/analysis. This is so that the user clearly
// understands that the inside instructions are repeated.
@@ -179,8 +162,8 @@ Expected<InstructionBenchmark> BenchmarkRunner::runConfiguration(
if (Error E = assembleToStream(
State.getExegesisTarget(), State.createTargetMachine(),
BC.LiveIns, BC.Key.RegisterInitialValues,
- Repetitor->Repeat(Instructions, MinInstructionsForSnippet,
- LoopBodySizeForSnippet),
+ Repetitor.Repeat(Instructions, MinInstructionsForSnippet,
+ LoopBodySizeForSnippet),
OS)) {
return std::move(E);
}
@@ -192,7 +175,7 @@ Expected<InstructionBenchmark> BenchmarkRunner::runConfiguration(
// Assemble NumRepetitions instructions repetitions of the snippet for
// measurements.
- const auto Filler = Repetitor->Repeat(
+ const auto Filler = Repetitor.Repeat(
Instructions, InstrBenchmark.NumRepetitions, LoopBodySize);
object::OwningBinary<object::ObjectFile> ObjectFile;
@@ -219,7 +202,7 @@ Expected<InstructionBenchmark> BenchmarkRunner::runConfiguration(
if (BenchmarkSkipMeasurements) {
InstrBenchmark.Error =
"in --skip-measurements mode, actual measurements skipped.";
- continue;
+ return InstrBenchmark;
}
const FunctionExecutorImpl Executor(State, std::move(ObjectFile),
@@ -239,31 +222,9 @@ Expected<InstructionBenchmark> BenchmarkRunner::runConfiguration(
BM.PerSnippetValue *= static_cast<double>(Instructions.size()) /
InstrBenchmark.NumRepetitions;
}
- if (InstrBenchmark.Measurements.empty()) {
- InstrBenchmark.Measurements = std::move(*NewMeasurements);
- continue;
- }
-
- assert(Repetitors.size() > 1 && !InstrBenchmark.Measurements.empty() &&
- "We're in an 'min' repetition mode, and need to aggregate new "
- "result to the existing result.");
- assert(InstrBenchmark.Measurements.size() == NewMeasurements->size() &&
- "Expected to have identical number of measurements.");
- for (auto I : zip(InstrBenchmark.Measurements, *NewMeasurements)) {
- BenchmarkMeasure &Measurement = std::get<0>(I);
- 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);
- }
+ InstrBenchmark.Measurements = std::move(*NewMeasurements);
}
- // We successfully measured everything, so don't discard the results.
- CBOR.disarm();
return InstrBenchmark;
}
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
index 870105d7bc14..3932407bd961 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
@@ -42,8 +42,7 @@ class BenchmarkRunner {
Expected<InstructionBenchmark>
runConfiguration(const BenchmarkCode &Configuration, unsigned NumRepetitions,
- unsigned LoopUnrollFactor,
- ArrayRef<std::unique_ptr<const SnippetRepetitor>> Repetitors,
+ unsigned LoopUnrollFactor, const SnippetRepetitor &Repetitor,
bool DumpObjectToDisk) const;
// Scratch space to run instructions that touch memory.
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index c4baf29ab72b..7f9cd3991cf5 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -406,8 +406,40 @@ void benchmarkMain() {
Meter.emplace(Configurations.size());
for (const BenchmarkCode &Conf : Configurations) {
ProgressMeter<>::ProgressMeterStep MeterStep(Meter ? &*Meter : nullptr);
- InstructionBenchmark Result = ExitOnErr(Runner->runConfiguration(
- Conf, NumRepetitions, LoopBodySize, Repetitors, DumpObjectToDisk));
+ SmallVector<InstructionBenchmark, 2> AllResults;
+
+ for (const std::unique_ptr<const SnippetRepetitor> &Repetitor :
+ Repetitors) {
+ AllResults.emplace_back(ExitOnErr(Runner->runConfiguration(
+ Conf, NumRepetitions, LoopBodySize, *Repetitor, DumpObjectToDisk)));
+ }
+ InstructionBenchmark &Result = AllResults.front();
+
+ if (RepetitionMode == InstructionBenchmark::RepetitionModeE::AggregateMin) {
+ assert(!Result.Measurements.empty() &&
+ "We're in an 'min' repetition mode, and need to aggregate new "
+ "result to the existing result.");
+ for (const InstructionBenchmark &OtherResult :
+ ArrayRef<InstructionBenchmark>(AllResults).drop_front()) {
+ llvm::append_range(Result.AssembledSnippet,
+ OtherResult.AssembledSnippet);
+ 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);
+ }
+ }
+ }
+
ExitOnFileError(BenchmarkFile, Result.writeYaml(State, BenchmarkFile));
}
exegesis::pfm::pfmTerminate();
More information about the llvm-commits
mailing list