From: Roman Lebedev Date: Sat, 17 Dec 2022 17:43:07 +0000 (+0300) Subject: [NFCI][llvm-exegesis] Extract 'Min' repetition handling from `BenchmarkRunner` into... X-Git-Tag: upstream/17.0.6~23389 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=17e202424c021fd903950fec7a8b6cca2d83abce;p=platform%2Fupstream%2Fllvm.git [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. --- diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp index 70e3911..f8da149 100644 --- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp +++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp @@ -137,8 +137,7 @@ private: Expected BenchmarkRunner::runConfiguration( const BenchmarkCode &BC, unsigned NumRepetitions, unsigned LoopBodySize, - ArrayRef> 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 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 &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 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 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 ObjectFile; @@ -219,7 +202,7 @@ Expected 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 BenchmarkRunner::runConfiguration( BM.PerSnippetValue *= static_cast(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 870105d..3932407 100644 --- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h +++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h @@ -42,8 +42,7 @@ public: Expected runConfiguration(const BenchmarkCode &Configuration, unsigned NumRepetitions, - unsigned LoopUnrollFactor, - ArrayRef> 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 c4baf29..7f9cd39 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 AllResults; + + for (const std::unique_ptr &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(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();