Skip to content
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

[llvm-exegesis] Fix snippet value scaling #77226

Conversation

boomanaiden154
Copy link
Contributor

Currently, BenchmarkRunner scales the per snippet counters by multiplying the raw counter values by the number of instructions (casted to a double) divided by the minimum number of instructions. This is incorrect for the loop repetition mode for snippets that don't fit a whole number of times into the minimum instruction count. For example, with 3 instructions in the snippet and the minimum number of instructions set to 4, the loop repetitor will execute a total of six instructions, but BenchmarkRunner will scale the raw count by 3/4 instead of 3/6=1/2. This will also be incorrect for the duplicate snippet repetitor after #77224.

This patch fixes this behavior by dividing the raw count by the ceiling of the number of repetitions divided by the instruction count.

Currently, BenchmarkRunner scales the per snippet counters by
multiplying the raw counter values by the number of instructions (casted
to a double) divided by the minimum number of instructions. This is
incorrect for the loop repetition mode for snippets that don't fit a
whole number of times into the minimum instruction count. For example,
with 3 instructions in the snippet and the minimum number of
instructions set to 4, the loop repetitor will execute a total of six
instructions, but BenchmarkRunner will scale the raw count by 3/4
instead of 3/6=1/2. This will also be incorrect for the duplicate
snippet repetitor after llvm#77224.

This patch fixes this behavior by dividing the raw count by the ceiling
of the number of repetitions divided by the instruction count.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 7, 2024

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

Author: Aiden Grossman (boomanaiden154)

Changes

Currently, BenchmarkRunner scales the per snippet counters by multiplying the raw counter values by the number of instructions (casted to a double) divided by the minimum number of instructions. This is incorrect for the loop repetition mode for snippets that don't fit a whole number of times into the minimum instruction count. For example, with 3 instructions in the snippet and the minimum number of instructions set to 4, the loop repetitor will execute a total of six instructions, but BenchmarkRunner will scale the raw count by 3/4 instead of 3/6=1/2. This will also be incorrect for the duplicate snippet repetitor after #77224.

This patch fixes this behavior by dividing the raw count by the ceiling of the number of repetitions divided by the instruction count.


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

1 Files Affected:

  • (modified) llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp (+4-3)
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index dee7af5fd520a4..87e2b768d5e63f 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <cmath>
 #include <memory>
 #include <string>
 
@@ -615,9 +616,9 @@ std::pair<Error, Benchmark> BenchmarkRunner::runConfiguration(
     // Scale the measurements by instruction.
     BM.PerInstructionValue /= BenchmarkResult.NumRepetitions;
     // Scale the measurements by snippet.
-    BM.PerSnippetValue *=
-        static_cast<double>(BenchmarkResult.Key.Instructions.size()) /
-        BenchmarkResult.NumRepetitions;
+    BM.PerSnippetValue /=
+        std::ceil(BenchmarkResult.NumRepetitions /
+                  static_cast<double>(BenchmarkResult.Key.Instructions.size()));
   }
   BenchmarkResult.Measurements = std::move(*NewMeasurements);
 

@boomanaiden154 boomanaiden154 merged commit 16b2387 into llvm:main Jan 16, 2024
4 of 5 checks passed
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Currently, BenchmarkRunner scales the per snippet counters by
multiplying the raw counter values by the number of instructions (casted
to a double) divided by the minimum number of instructions. This is
incorrect for the loop repetition mode for snippets that don't fit a
whole number of times into the minimum instruction count. For example,
with 3 instructions in the snippet and the minimum number of
instructions set to 4, the loop repetitor will execute a total of six
instructions, but BenchmarkRunner will scale the raw count by 3/4
instead of 3/6=1/2. This will also be incorrect for the duplicate
snippet repetitor after llvm#77224.

This patch fixes this behavior by dividing the raw count by the ceiling
of the number of repetitions divided by the instruction count.
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.

None yet

3 participants