[clang] [clang-tools-extra] [llvm] [llvm-exegesis] Replace --num-repetitions with --min-instructions (PR #77153)
Aiden Grossman via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 1 00:31:14 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/3] [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 02c4da11e032d..d13409fe4738a 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 0d08febae20cb..ffba634151586 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 dee7af5fd520a..6c5f9e557812d 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 d746a0f775646..26b275a5658e8 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 ffbf94ce0fcb2..6d048e8188780 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 201e0a8e7acce..e83e661fa9a04 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 616f7bac54bc4..7aed4922d58d5 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/3] 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 d13409fe4738a..a4ce8ee810ccd 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);
}
};
>From 7af9412c5707e6fd2ffbc4d001780f05db2474e6 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 5 Jan 2024 14:53:40 -0800
Subject: [PATCH 3/3] Update docs
---
llvm/docs/CommandGuide/llvm-exegesis.rst | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/llvm/docs/CommandGuide/llvm-exegesis.rst b/llvm/docs/CommandGuide/llvm-exegesis.rst
index 2ee533c324d96..59c582efaf9fa 100644
--- a/llvm/docs/CommandGuide/llvm-exegesis.rst
+++ b/llvm/docs/CommandGuide/llvm-exegesis.rst
@@ -304,22 +304,22 @@ OPTIONS
.. option:: --repetition-mode=[duplicate|loop|min]
Specify the repetition mode. `duplicate` will create a large, straight line
- basic block with `num-repetitions` instructions (repeating the snippet
- `num-repetitions`/`snippet size` times). `loop` will, optionally, duplicate the
+ basic block with `min-instructions` instructions (repeating the snippet
+ `min-instructions`/`snippet size` times). `loop` will, optionally, duplicate the
snippet until the loop body contains at least `loop-body-size` instructions,
- and then wrap the result in a loop which will execute `num-repetitions`
+ and then wrap the result in a loop which will execute `min-instructions`
instructions (thus, again, repeating the snippet
- `num-repetitions`/`snippet size` times). The `loop` mode, especially with loop
+ `min-instructions`/`snippet size` times). The `loop` mode, especially with loop
unrolling tends to better hide the effects of the CPU frontend on architectures
that cache decoded instructions, but consumes a register for counting
iterations. If performing an analysis over many opcodes, it may be best to
instead use the `min` mode, which will run each other mode,
and produce the minimal measured result.
-.. option:: --num-repetitions=<Number of repetitions>
+.. option:: --min-instructions=<Number of instructions>
Specify the target number of executed instructions. Note that the actual
- repetition count of the snippet will be `num-repetitions`/`snippet size`.
+ repetition count of the snippet will be `min-instructions`/`snippet size`.
Higher values lead to more accurate measurements but lengthen the benchmark.
.. option:: --loop-body-size=<Preferred loop body size>
More information about the cfe-commits
mailing list