Skip to content

[NFC][llvm-exegesis] Refactor InstrBenchmark to BenchmarkResult #76388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

boomanaiden154
Copy link
Contributor

This patch refactors InstrBenchmark to BenchmarkResult. Most of the renaming away from things prefixed with Instr was performed in a previous commit, but this specific instance was missed.

This patch refactors InstrBenchmark to BenchmarkResult. Most of the
renaming away from things prefixed with Instr was performed in a
previous commit, but this specific instance was missed.
@llvmbot
Copy link
Member

llvmbot commented Dec 26, 2023

@llvm/pr-subscribers-tools-llvm-exegesis

Author: Aiden Grossman (boomanaiden154)

Changes

This patch refactors InstrBenchmark to BenchmarkResult. Most of the renaming away from things prefixed with Instr was performed in a previous commit, but this specific instance was missed.


Full diff: https://github.com/llvm/llvm-project/pull/76388.diff

3 Files Affected:

  • (modified) llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp (+25-23)
  • (modified) llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h (+1-1)
  • (modified) llvm/tools/llvm-exegesis/llvm-exegesis.cpp (+3-3)
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index 12e774e1a4b8d6..1ee59a86ebbdcf 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -499,19 +499,20 @@ BenchmarkRunner::getRunnableConfiguration(
     const SnippetRepetitor &Repetitor) const {
   RunnableConfiguration RC;
 
-  Benchmark &InstrBenchmark = RC.InstrBenchmark;
-  InstrBenchmark.Mode = Mode;
-  InstrBenchmark.CpuName = std::string(State.getTargetMachine().getTargetCPU());
-  InstrBenchmark.LLVMTriple =
+  Benchmark &BenchmarkResult = RC.BenchmarkResult;
+  BenchmarkResult.Mode = Mode;
+  BenchmarkResult.CpuName =
+      std::string(State.getTargetMachine().getTargetCPU());
+  BenchmarkResult.LLVMTriple =
       State.getTargetMachine().getTargetTriple().normalize();
-  InstrBenchmark.NumRepetitions = NumRepetitions;
-  InstrBenchmark.Info = BC.Info;
+  BenchmarkResult.NumRepetitions = NumRepetitions;
+  BenchmarkResult.Info = BC.Info;
 
   const std::vector<MCInst> &Instructions = BC.Key.Instructions;
 
   bool GenerateMemoryInstructions = ExecutionMode == ExecutionModeE::SubProcess;
 
-  InstrBenchmark.Key = BC.Key;
+  BenchmarkResult.Key = BC.Key;
 
   // Assemble at least kMinInstructionsForSnippet instructions by repeating
   // the snippet for debug/analysis. This is so that the user clearly
@@ -526,7 +527,7 @@ BenchmarkRunner::getRunnableConfiguration(
       return std::move(E);
 
     if (auto Err = getBenchmarkFunctionBytes(*Snippet,
-                                             InstrBenchmark.AssembledSnippet))
+                                             BenchmarkResult.AssembledSnippet))
       return std::move(Err);
   }
 
@@ -534,8 +535,9 @@ BenchmarkRunner::getRunnableConfiguration(
   // measurements.
   if (BenchmarkPhaseSelector >
       BenchmarkPhaseSelectorE::PrepareAndAssembleSnippet) {
-    auto Snippet = assembleSnippet(BC, Repetitor, InstrBenchmark.NumRepetitions,
-                                   LoopBodySize, GenerateMemoryInstructions);
+    auto Snippet =
+        assembleSnippet(BC, Repetitor, BenchmarkResult.NumRepetitions,
+                        LoopBodySize, GenerateMemoryInstructions);
     if (Error E = Snippet.takeError())
       return std::move(E);
     RC.ObjectFile = getObjectFromBuffer(*Snippet);
@@ -577,7 +579,7 @@ BenchmarkRunner::createFunctionExecutor(
 std::pair<Error, Benchmark> BenchmarkRunner::runConfiguration(
     RunnableConfiguration &&RC,
     const std::optional<StringRef> &DumpFile) const {
-  Benchmark &InstrBenchmark = RC.InstrBenchmark;
+  Benchmark &BenchmarkResult = RC.BenchmarkResult;
   object::OwningBinary<object::ObjectFile> &ObjectFile = RC.ObjectFile;
 
   if (DumpFile && BenchmarkPhaseSelector >
@@ -585,38 +587,38 @@ std::pair<Error, Benchmark> BenchmarkRunner::runConfiguration(
     auto ObjectFilePath =
         writeObjectFile(ObjectFile.getBinary()->getData(), *DumpFile);
     if (Error E = ObjectFilePath.takeError()) {
-      return {std::move(E), std::move(InstrBenchmark)};
+      return {std::move(E), std::move(BenchmarkResult)};
     }
     outs() << "Check generated assembly with: /usr/bin/objdump -d "
            << *ObjectFilePath << "\n";
   }
 
   if (BenchmarkPhaseSelector < BenchmarkPhaseSelectorE::Measure) {
-    InstrBenchmark.Error = "actual measurements skipped.";
-    return {Error::success(), std::move(InstrBenchmark)};
+    BenchmarkResult.Error = "actual measurements skipped.";
+    return {Error::success(), std::move(BenchmarkResult)};
   }
 
   Expected<std::unique_ptr<BenchmarkRunner::FunctionExecutor>> Executor =
-      createFunctionExecutor(std::move(ObjectFile), RC.InstrBenchmark.Key);
+      createFunctionExecutor(std::move(ObjectFile), RC.BenchmarkResult.Key);
   if (!Executor)
-    return {Executor.takeError(), std::move(InstrBenchmark)};
+    return {Executor.takeError(), std::move(BenchmarkResult)};
   auto NewMeasurements = runMeasurements(**Executor);
 
   if (Error E = NewMeasurements.takeError()) {
-    return {std::move(E), std::move(InstrBenchmark)};
+    return {std::move(E), std::move(BenchmarkResult)};
   }
-  assert(InstrBenchmark.NumRepetitions > 0 && "invalid NumRepetitions");
+  assert(BenchmarkResult.NumRepetitions > 0 && "invalid NumRepetitions");
   for (BenchmarkMeasure &BM : *NewMeasurements) {
     // Scale the measurements by instruction.
-    BM.PerInstructionValue /= InstrBenchmark.NumRepetitions;
+    BM.PerInstructionValue /= BenchmarkResult.NumRepetitions;
     // Scale the measurements by snippet.
     BM.PerSnippetValue *=
-        static_cast<double>(InstrBenchmark.Key.Instructions.size()) /
-        InstrBenchmark.NumRepetitions;
+        static_cast<double>(BenchmarkResult.Key.Instructions.size()) /
+        BenchmarkResult.NumRepetitions;
   }
-  InstrBenchmark.Measurements = std::move(*NewMeasurements);
+  BenchmarkResult.Measurements = std::move(*NewMeasurements);
 
-  return {Error::success(), std::move(InstrBenchmark)};
+  return {Error::success(), std::move(BenchmarkResult)};
 }
 
 Expected<std::string>
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
index 2c48d07e37ca9f..d746a0f775646f 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h
@@ -56,7 +56,7 @@ class BenchmarkRunner {
   private:
     RunnableConfiguration() = default;
 
-    Benchmark InstrBenchmark;
+    Benchmark BenchmarkResult;
     object::OwningBinary<object::ObjectFile> ObjectFile;
   };
 
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index a5f8a09dcb241d..1b35fde815f11f 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -410,7 +410,7 @@ static void runBenchmarkConfigurations(
       std::optional<StringRef> DumpFile;
       if (DumpObjectToDisk.getNumOccurrences())
         DumpFile = DumpObjectToDisk;
-      auto [Err, InstrBenchmark] =
+      auto [Err, BenchmarkResult] =
           Runner.runConfiguration(std::move(RC), DumpFile);
       if (Err) {
         // Errors from executing the snippets are fine.
@@ -419,9 +419,9 @@ static void runBenchmarkConfigurations(
           llvm::errs() << "llvm-exegesis error: " << toString(std::move(Err));
           exit(1);
         }
-        InstrBenchmark.Error = toString(std::move(Err));
+        BenchmarkResult.Error = toString(std::move(Err));
       }
-      AllResults.push_back(std::move(InstrBenchmark));
+      AllResults.push_back(std::move(BenchmarkResult));
     }
     Benchmark &Result = AllResults.front();
 

@boomanaiden154 boomanaiden154 merged commit 3f3c5e5 into llvm:main Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants