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] Add matcher for register initial values #76666

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

boomanaiden154
Copy link
Contributor

Currently, the unit tests for the BenchmarkResult struct do not check if the register initial values can be parsed back in. This patch adds a matcher and tests that the register initial values can be parsed correctly. This exercises code already contained within the benchmark to yaml infrastructure.

Currently, the unit tests for the BenchmarkResult struct do not check if
the register initial values can be parsed back in. This patch adds a
matcher and tests that the register initial values can be parsed
correctly. This exercises code already contained within the benchmark to
yaml infrastructure.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 1, 2024

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

Author: Aiden Grossman (boomanaiden154)

Changes

Currently, the unit tests for the BenchmarkResult struct do not check if the register initial values can be parsed back in. This patch adds a matcher and tests that the register initial values can be parsed correctly. This exercises code already contained within the benchmark to yaml infrastructure.


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

1 Files Affected:

  • (modified) llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp (+13)
diff --git a/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp
index 6c558b59be982d..616f7bac54bc43 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/X86/BenchmarkResultTest.cpp
@@ -46,6 +46,15 @@ MATCHER(EqMCInst, "") {
   return true;
 }
 
+MATCHER(EqRegValue, "") {
+  const RegisterValue Lhs = get<0>(arg);
+  const RegisterValue Rhs = get<1>(arg);
+  if (Lhs.Register != Rhs.Register || Lhs.Value != Rhs.Value)
+    return false;
+
+  return true;
+}
+
 namespace {
 
 TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
@@ -120,6 +129,8 @@ TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
     EXPECT_THAT(FromDisk.Key.Instructions,
                 Pointwise(EqMCInst(), ToDisk.Key.Instructions));
     EXPECT_EQ(FromDisk.Key.Config, ToDisk.Key.Config);
+    EXPECT_THAT(FromDisk.Key.RegisterInitialValues,
+                Pointwise(EqRegValue(), ToDisk.Key.RegisterInitialValues));
     EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
     EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);
     EXPECT_EQ(FromDisk.LLVMTriple, ToDisk.LLVMTriple);
@@ -137,6 +148,8 @@ TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
     EXPECT_THAT(FromDisk.Key.Instructions,
                 Pointwise(EqMCInst(), ToDisk.Key.Instructions));
     EXPECT_EQ(FromDisk.Key.Config, ToDisk.Key.Config);
+    EXPECT_THAT(FromDisk.Key.RegisterInitialValues,
+                Pointwise(EqRegValue(), ToDisk.Key.RegisterInitialValues));
     EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
     EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);
     EXPECT_EQ(FromDisk.LLVMTriple, ToDisk.LLVMTriple);

@boomanaiden154
Copy link
Contributor Author

I don't believe this matters for any functional use of the tool as the YAML parsing should only be needed in analysis mode which I don't believe depends on the initial register values, but it still seemed like a gap in the test coverage when I was working on YAML support for memory annotations.

Copy link
Contributor

@legrosbuffle legrosbuffle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@boomanaiden154 boomanaiden154 merged commit d9c8edf into llvm:main Jan 5, 2024
5 checks passed
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Currently, the unit tests for the BenchmarkResult struct do not check if
the register initial values can be parsed back in. This patch adds a
matcher and tests that the register initial values can be parsed
correctly. This exercises code already contained within the benchmark to
yaml infrastructure.
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