[llvm] [llvm-exegesis] Replace --num-repetitions with --min-instructions (PR #77153)
Aiden Grossman via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 5 14:48:11 PST 2024
https://github.com/boomanaiden154 updated https://github.com/llvm/llvm-project/pull/77153
>From 1a86a03fdc9d6eee08830ff2f113c39e6096d564 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 5 Jan 2024 14:40:26 -0800
Subject: [PATCH 1/2] [llvm-exegesis] Replace --num-repetitions with
--min-instructions
This patch replaces --num-repetitions with --min-instructions to make it
more clear that the value refers to the minimum number of instructions
in the final assembled snippet rather than the number of repetitions of
the snippet. This patch also refactors some llvm-exegesis internal
variable names to reflect the name change.
Fixes #76890.
---
llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp | 8 +++++++-
llvm/tools/llvm-exegesis/lib/BenchmarkResult.h | 2 +-
llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp | 16 ++++++++--------
llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h | 2 +-
llvm/tools/llvm-exegesis/llvm-exegesis.cpp | 13 +++++++------
.../llvm-exegesis/Mips/BenchmarkResultTest.cpp | 6 +++---
.../llvm-exegesis/X86/BenchmarkResultTest.cpp | 6 +++---
7 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
index 02c4da11e032d6..d13409fe4738af 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
@@ -293,7 +293,6 @@ struct MappingContextTraits<exegesis::Benchmark, YamlContext> {
Io.mapRequired("key", Obj.Key, Context);
Io.mapRequired("cpu_name", Obj.CpuName);
Io.mapRequired("llvm_triple", Obj.LLVMTriple);
- Io.mapRequired("num_repetitions", Obj.NumRepetitions);
Io.mapRequired("measurements", Obj.Measurements);
Io.mapRequired("error", Obj.Error);
Io.mapOptional("info", Obj.Info);
@@ -301,6 +300,13 @@ struct MappingContextTraits<exegesis::Benchmark, YamlContext> {
MappingNormalization<NormalizedBinary, std::vector<uint8_t>> BinaryString(
Io, Obj.AssembledSnippet);
Io.mapOptional("assembled_snippet", BinaryString->Binary);
+ // Optionally map num_repetitions and min_instructions to the same
+ // value to preserve backwards compatibility.
+ // TODO(boomanaiden154): Move min_instructions to mapRequired and
+ // remove num_repetitions once num_repetitions is ready to be removed
+ // completely.
+ Io.mapOptional("num_repetitions", Obj.MinInstructions);
+ Io.mapOptional("min_instructions", Obj.MinInstructions);
}
};
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
index 0d08febae20cb3..ffba634151586b 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
@@ -100,7 +100,7 @@ struct Benchmark {
const MCInst &keyInstruction() const { return Key.Instructions[0]; }
// 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;
+ unsigned MinInstructions = 0;
enum RepetitionModeE { Duplicate, Loop, AggregateMin };
// Note that measurements are per instruction.
std::vector<BenchmarkMeasure> Measurements;
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index dee7af5fd520a4..6c5f9e557812d3 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -498,7 +498,7 @@ Expected<SmallString<0>> BenchmarkRunner::assembleSnippet(
Expected<BenchmarkRunner::RunnableConfiguration>
BenchmarkRunner::getRunnableConfiguration(
- const BenchmarkCode &BC, unsigned NumRepetitions, unsigned LoopBodySize,
+ const BenchmarkCode &BC, unsigned MinInstructions, unsigned LoopBodySize,
const SnippetRepetitor &Repetitor) const {
RunnableConfiguration RC;
@@ -508,7 +508,7 @@ BenchmarkRunner::getRunnableConfiguration(
std::string(State.getTargetMachine().getTargetCPU());
BenchmarkResult.LLVMTriple =
State.getTargetMachine().getTargetTriple().normalize();
- BenchmarkResult.NumRepetitions = NumRepetitions;
+ BenchmarkResult.MinInstructions = MinInstructions;
BenchmarkResult.Info = BC.Info;
const std::vector<MCInst> &Instructions = BC.Key.Instructions;
@@ -534,12 +534,12 @@ BenchmarkRunner::getRunnableConfiguration(
return std::move(Err);
}
- // Assemble NumRepetitions instructions repetitions of the snippet for
- // measurements.
+ // Assemble enough repetitions of the snippet so we have at least
+ // MinInstructios instructions.
if (BenchmarkPhaseSelector >
BenchmarkPhaseSelectorE::PrepareAndAssembleSnippet) {
auto Snippet =
- assembleSnippet(BC, Repetitor, BenchmarkResult.NumRepetitions,
+ assembleSnippet(BC, Repetitor, BenchmarkResult.MinInstructions,
LoopBodySize, GenerateMemoryInstructions);
if (Error E = Snippet.takeError())
return std::move(E);
@@ -610,14 +610,14 @@ std::pair<Error, Benchmark> BenchmarkRunner::runConfiguration(
if (Error E = NewMeasurements.takeError()) {
return {std::move(E), std::move(BenchmarkResult)};
}
- assert(BenchmarkResult.NumRepetitions > 0 && "invalid NumRepetitions");
+ assert(BenchmarkResult.MinInstructions > 0 && "invalid MinInstructions");
for (BenchmarkMeasure &BM : *NewMeasurements) {
// Scale the measurements by instruction.
- BM.PerInstructionValue /= BenchmarkResult.NumRepetitions;
+ BM.PerInstructionValue /= BenchmarkResult.MinInstructions;
// Scale the measurements by snippet.
BM.PerSnippetValue *=
static_cast<double>(BenchmarkResult.Key.Instructions.size()) /
- BenchmarkResult.NumRepetitions;
+ BenchmarkResult.MinInstructions;
}
BenchmarkResult.Measurements = std::move(*NewMeasurements);
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
index d746a0f775646f..26b275a5658e80 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
@@ -62,7 +62,7 @@ class BenchmarkRunner {
Expected<RunnableConfiguration>
getRunnableConfiguration(const BenchmarkCode &Configuration,
- unsigned NumRepetitions, unsigned LoopUnrollFactor,
+ unsigned MinInstructions, unsigned LoopUnrollFactor,
const SnippetRepetitor &Repetitor) const;
std::pair<Error, Benchmark>
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index ffbf94ce0fcb26..6d048e81887808 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -148,9 +148,10 @@ static cl::opt<bool>
cl::cat(BenchmarkOptions), cl::init(false));
static cl::opt<unsigned>
- NumRepetitions("num-repetitions",
- cl::desc("number of time to repeat the asm snippet"),
- cl::cat(BenchmarkOptions), cl::init(10000));
+ MinInstructions("min-instructions",
+ cl::desc("The minimum number of instructions that should "
+ "be included in the snippet"),
+ cl::cat(BenchmarkOptions), cl::init(10000));
static cl::opt<unsigned>
LoopBodySize("loop-body-size",
@@ -406,7 +407,7 @@ static void runBenchmarkConfigurations(
for (const std::unique_ptr<const SnippetRepetitor> &Repetitor :
Repetitors) {
auto RC = ExitOnErr(Runner.getRunnableConfiguration(
- Conf, NumRepetitions, LoopBodySize, *Repetitor));
+ Conf, MinInstructions, LoopBodySize, *Repetitor));
std::optional<StringRef> DumpFile;
if (DumpObjectToDisk.getNumOccurrences())
DumpFile = DumpObjectToDisk;
@@ -557,9 +558,9 @@ void benchmarkMain() {
}
}
- if (NumRepetitions == 0) {
+ if (MinInstructions == 0) {
ExitOnErr.setBanner("llvm-exegesis: ");
- ExitWithError("--num-repetitions must be greater than zero");
+ ExitWithError("--min-instructions must ee greater than zero");
}
// Write to standard output if file is not set.
diff --git a/llvm/unittests/tools/llvm-exegesis/Mips/BenchmarkResultTest.cpp b/llvm/unittests/tools/llvm-exegesis/Mips/BenchmarkResultTest.cpp
index 201e0a8e7acce2..e83e661fa9a04c 100644
--- a/llvm/unittests/tools/llvm-exegesis/Mips/BenchmarkResultTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/Mips/BenchmarkResultTest.cpp
@@ -64,7 +64,7 @@ TEST_F(MipsBenchmarkResultTest, WriteToAndReadFromDisk) {
ToDisk.Mode = Benchmark::Latency;
ToDisk.CpuName = "cpu_name";
ToDisk.LLVMTriple = "llvm_triple";
- ToDisk.NumRepetitions = 1;
+ ToDisk.MinInstructions = 1;
ToDisk.Measurements.push_back(BenchmarkMeasure{"a", 1, 1});
ToDisk.Measurements.push_back(BenchmarkMeasure{"b", 2, 2});
ToDisk.Error = "error";
@@ -98,7 +98,7 @@ TEST_F(MipsBenchmarkResultTest, WriteToAndReadFromDisk) {
EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);
EXPECT_EQ(FromDisk.LLVMTriple, ToDisk.LLVMTriple);
- EXPECT_EQ(FromDisk.NumRepetitions, ToDisk.NumRepetitions);
+ EXPECT_EQ(FromDisk.MinInstructions, ToDisk.MinInstructions);
EXPECT_THAT(FromDisk.Measurements, ToDisk.Measurements);
EXPECT_THAT(FromDisk.Error, ToDisk.Error);
EXPECT_EQ(FromDisk.Info, ToDisk.Info);
@@ -115,7 +115,7 @@ TEST_F(MipsBenchmarkResultTest, WriteToAndReadFromDisk) {
EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);
EXPECT_EQ(FromDisk.LLVMTriple, ToDisk.LLVMTriple);
- EXPECT_EQ(FromDisk.NumRepetitions, ToDisk.NumRepetitions);
+ EXPECT_EQ(FromDisk.MinInstructions, ToDisk.MinInstructions);
EXPECT_THAT(FromDisk.Measurements, ToDisk.Measurements);
EXPECT_THAT(FromDisk.Error, ToDisk.Error);
EXPECT_EQ(FromDisk.Info, ToDisk.Info);
diff --git a/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp
index 616f7bac54bc43..7aed4922d58d5c 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp
@@ -83,7 +83,7 @@ TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
ToDisk.Mode = Benchmark::Latency;
ToDisk.CpuName = "cpu_name";
ToDisk.LLVMTriple = "llvm_triple";
- ToDisk.NumRepetitions = 1;
+ ToDisk.MinInstructions = 1;
ToDisk.Measurements.push_back(BenchmarkMeasure{"a", 1, 1});
ToDisk.Measurements.push_back(BenchmarkMeasure{"b", 2, 2});
ToDisk.Error = "error";
@@ -134,7 +134,7 @@ TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);
EXPECT_EQ(FromDisk.LLVMTriple, ToDisk.LLVMTriple);
- EXPECT_EQ(FromDisk.NumRepetitions, ToDisk.NumRepetitions);
+ EXPECT_EQ(FromDisk.MinInstructions, ToDisk.MinInstructions);
EXPECT_THAT(FromDisk.Measurements, ToDisk.Measurements);
EXPECT_THAT(FromDisk.Error, ToDisk.Error);
EXPECT_EQ(FromDisk.Info, ToDisk.Info);
@@ -153,7 +153,7 @@ TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);
EXPECT_EQ(FromDisk.LLVMTriple, ToDisk.LLVMTriple);
- EXPECT_EQ(FromDisk.NumRepetitions, ToDisk.NumRepetitions);
+ EXPECT_EQ(FromDisk.MinInstructions, ToDisk.MinInstructions);
EXPECT_THAT(FromDisk.Measurements, ToDisk.Measurements);
EXPECT_THAT(FromDisk.Error, ToDisk.Error);
EXPECT_EQ(FromDisk.Info, ToDisk.Info);
>From da9653ece981f5f2bf86e497dccb3e66fc6e2ec2 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 5 Jan 2024 14:48:02 -0800
Subject: [PATCH 2/2] Fix yaml output/input
---
.../llvm-exegesis/lib/BenchmarkResult.cpp | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
index d13409fe4738af..a4ce8ee810ccd7 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
@@ -293,6 +293,17 @@ struct MappingContextTraits<exegesis::Benchmark, YamlContext> {
Io.mapRequired("key", Obj.Key, Context);
Io.mapRequired("cpu_name", Obj.CpuName);
Io.mapRequired("llvm_triple", Obj.LLVMTriple);
+ // Optionally map num_repetitions and min_instructions to the same
+ // value to preserve backwards compatibility.
+ // TODO(boomanaiden154): Move min_instructions to mapRequired and
+ // remove num_repetitions once num_repetitions is ready to be removed
+ // completely.
+ if (Io.outputting())
+ Io.mapRequired("min_instructions", Obj.MinInstructions);
+ else {
+ Io.mapOptional("num_repetitions", Obj.MinInstructions);
+ Io.mapOptional("min_instructions", Obj.MinInstructions);
+ }
Io.mapRequired("measurements", Obj.Measurements);
Io.mapRequired("error", Obj.Error);
Io.mapOptional("info", Obj.Info);
@@ -300,13 +311,6 @@ struct MappingContextTraits<exegesis::Benchmark, YamlContext> {
MappingNormalization<NormalizedBinary, std::vector<uint8_t>> BinaryString(
Io, Obj.AssembledSnippet);
Io.mapOptional("assembled_snippet", BinaryString->Binary);
- // Optionally map num_repetitions and min_instructions to the same
- // value to preserve backwards compatibility.
- // TODO(boomanaiden154): Move min_instructions to mapRequired and
- // remove num_repetitions once num_repetitions is ready to be removed
- // completely.
- Io.mapOptional("num_repetitions", Obj.MinInstructions);
- Io.mapOptional("min_instructions", Obj.MinInstructions);
}
};
More information about the llvm-commits
mailing list