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] Remove unused Counter::read method #76651

Merged

Conversation

boomanaiden154
Copy link
Contributor

This method was simply a wrapper around readOrError. All users within the llvm-exegesis code base should have been processing an actual error rather than using the wrapper. This patch removes the wrapper and rewrites the users (just 1) to use the readOrError method.

This method was simply a wrapper around readOrError. All users within
the llvm-exegesis code base should have been processing an actual error
rather than using the wrapper. This patch removes the wrapper and
rewrites the users (just 1) to use the readOrError method.
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 31, 2023

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

Author: Aiden Grossman (boomanaiden154)

Changes

This method was simply a wrapper around readOrError. All users within the llvm-exegesis code base should have been processing an actual error rather than using the wrapper. This patch removes the wrapper and rewrites the users (just 1) to use the readOrError method.


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

3 Files Affected:

  • (modified) llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp (+5-1)
  • (modified) llvm/tools/llvm-exegesis/lib/PerfHelper.cpp (-13)
  • (modified) llvm/tools/llvm-exegesis/lib/PerfHelper.h (-3)
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index 1ee59a86ebbdcf..c57fce970b2139 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -356,7 +356,11 @@ class SubProcessFunctionExecutorImpl
       if (ChildExitCode == 0) {
         // The child exited succesfully, read counter values and return
         // success
-        CounterValues[0] = Counter->read();
+        auto CounterValueOrErr = Counter->readOrError();
+        if (!CounterValueOrErr)
+          return CounterValueOrErr.takeError();
+        CounterValues.swap(*CounterValueOrErr);
+
         return Error::success();
       }
       // The child exited, but not successfully
diff --git a/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp b/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp
index 3ff1745e9e0629..314de1ec32366f 100644
--- a/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp
+++ b/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp
@@ -148,17 +148,6 @@ void Counter::stop() {
     ioctl(FileDescriptor, PERF_EVENT_IOC_DISABLE, 0);
 }
 
-int64_t Counter::read() const {
-  auto ValueOrError = readOrError();
-  if (ValueOrError) {
-    if (!ValueOrError.get().empty())
-      return ValueOrError.get()[0];
-    errs() << "Counter has no reading\n";
-  } else
-    errs() << ValueOrError.takeError() << "\n";
-  return -1;
-}
-
 llvm::Expected<llvm::SmallVector<int64_t, 4>>
 Counter::readOrError(StringRef /*unused*/) const {
   int64_t Count = 0;
@@ -187,8 +176,6 @@ void Counter::start() {}
 
 void Counter::stop() {}
 
-int64_t Counter::read() const { return 42; }
-
 llvm::Expected<llvm::SmallVector<int64_t, 4>>
 Counter::readOrError(StringRef /*unused*/) const {
   if (IsDummyEvent) {
diff --git a/llvm/tools/llvm-exegesis/lib/PerfHelper.h b/llvm/tools/llvm-exegesis/lib/PerfHelper.h
index a50974f0a67be9..894aac1f197ed1 100644
--- a/llvm/tools/llvm-exegesis/lib/PerfHelper.h
+++ b/llvm/tools/llvm-exegesis/lib/PerfHelper.h
@@ -95,9 +95,6 @@ class Counter {
   /// Stops the measurement of the event.
   void stop();
 
-  /// Returns the current value of the counter or -1 if it cannot be read.
-  int64_t read() const;
-
   /// Returns the current value of the counter or error if it cannot be read.
   /// FunctionBytes: The benchmark function being executed.
   /// This is used to filter out the measurements to ensure they are only

@boomanaiden154 boomanaiden154 merged commit 6f4cc13 into main Jan 5, 2024
3 of 4 checks passed
@boomanaiden154 boomanaiden154 deleted the users/boomanaiden154/exegesis-validation-counters-no-read branch January 5, 2024 21:31
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
This method was simply a wrapper around readOrError. All users within
the llvm-exegesis code base should have been processing an actual error
rather than using the wrapper. This patch removes the wrapper and
rewrites the users (just 1) to use the readOrError method.
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