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] Refactor individual counter data to ConfiguredEvent #77900

Conversation

boomanaiden154
Copy link
Contributor

This further sets things up for validation events. Having a separate abstraction for a configured event that is setup as a counter allows for much easier creation of more events in the future within a single counter group (like validation counters) without duplicating any code.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 12, 2024

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

Author: Aiden Grossman (boomanaiden154)

Changes

This further sets things up for validation events. Having a separate abstraction for a configured event that is setup as a counter allows for much easier creation of more events in the future within a single counter group (like validation counters) without duplicating any code.


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

3 Files Affected:

  • (modified) llvm/tools/llvm-exegesis/lib/PerfHelper.cpp (+50-30)
  • (modified) llvm/tools/llvm-exegesis/lib/PerfHelper.h (+27-5)
  • (modified) llvm/tools/llvm-exegesis/lib/X86/X86Counter.cpp (+4-4)
diff --git a/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp b/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp
index f6e091bdff9aec..1dab8809d30280 100644
--- a/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp
+++ b/llvm/tools/llvm-exegesis/lib/PerfHelper.cpp
@@ -107,21 +107,18 @@ StringRef PerfEvent::getPfmEventString() const {
   return FullQualifiedEventString;
 }
 
-CounterGroup::CounterGroup(PerfEvent &&E, pid_t ProcessID)
-    : Event(std::move(E)) {
+ConfiguredEvent::ConfiguredEvent(PerfEvent &&EventToConfigure)
+    : Event(std::move(EventToConfigure)) {
   assert(Event.valid());
-  IsDummyEvent = Event.name() == PerfEvent::DummyEventString;
-  if (!IsDummyEvent)
-    initRealEvent(E, ProcessID);
 }
 
 #ifdef HAVE_LIBPFM
-void CounterGroup::initRealEvent(const PerfEvent &E, pid_t ProcessID) {
-  const int Cpu = -1;     // measure any processor.
-  const int GroupFd = -1; // no grouping of counters.
+void ConfiguredEvent::initRealEvent(const pid_t ProcessID) {
+  const int CPU = -1;
+  const int GroupFD = -1;
   const uint32_t Flags = 0;
   perf_event_attr AttrCopy = *Event.attribute();
-  FileDescriptor = perf_event_open(&AttrCopy, ProcessID, Cpu, GroupFd, Flags);
+  FileDescriptor = perf_event_open(&AttrCopy, ProcessID, CPU, GroupFD, Flags);
   if (FileDescriptor == -1) {
     errs() << "Unable to open event. ERRNO: " << strerror(errno)
            << ". Make sure your kernel allows user "
@@ -134,44 +131,67 @@ void CounterGroup::initRealEvent(const PerfEvent &E, pid_t ProcessID) {
   assert(FileDescriptor != -1 && "Unable to open event");
 }
 
-CounterGroup::~CounterGroup() {
+Expected<SmallVector<int64_t>>
+ConfiguredEvent::readOrError(StringRef /*unused*/) const {
+  int64_t Count = 0;
+  ssize_t ReadSize = ::read(FileDescriptor, &Count, sizeof(Count));
+
+  if (ReadSize != sizeof(Count))
+    return llvm::make_error<llvm::StringError>("Failed to read event counter",
+                                               llvm::errc::io_error);
+
+  SmallVector<int64_t, 1> Result;
+  Result.push_back(Count);
+  return Result;
+}
+
+ConfiguredEvent::~ConfiguredEvent() { close(FileDescriptor); }
+#else
+void ConfiguredEvent::initRealEvent(pid_t ProcessID) {}
+
+Expected<SmallVector<int64_t>>
+ConfiguredEvent::readOrError(StringRef /*unused*/) const {
+  return make_error<StringError>("Not implemented",
+                                 errc::function_not_supported);
+}
+
+ConfiguredEvent::~ConfiguredEvent() = default;
+#endif // HAVE_LIBPFM
+
+CounterGroup::CounterGroup(PerfEvent &&E, pid_t ProcessID)
+    : EventCounter(std::move(E)) {
+  IsDummyEvent = EventCounter.isDummyEvent();
   if (!IsDummyEvent)
-    close(FileDescriptor);
+    initRealEvent(ProcessID);
+}
+
+#ifdef HAVE_LIBPFM
+void CounterGroup::initRealEvent(pid_t ProcessID) {
+  EventCounter.initRealEvent(ProcessID);
 }
 
 void CounterGroup::start() {
   if (!IsDummyEvent)
-    ioctl(FileDescriptor, PERF_EVENT_IOC_RESET, 0);
+    ioctl(getFileDescriptor(), PERF_EVENT_IOC_RESET, 0);
 }
 
 void CounterGroup::stop() {
   if (!IsDummyEvent)
-    ioctl(FileDescriptor, PERF_EVENT_IOC_DISABLE, 0);
+    ioctl(getFileDescriptor(), PERF_EVENT_IOC_DISABLE, 0);
 }
 
 llvm::Expected<llvm::SmallVector<int64_t, 4>>
-CounterGroup::readOrError(StringRef /*unused*/) const {
-  int64_t Count = 0;
-  if (!IsDummyEvent) {
-    ssize_t ReadSize = ::read(FileDescriptor, &Count, sizeof(Count));
-    if (ReadSize != sizeof(Count))
-      return llvm::make_error<llvm::StringError>("Failed to read event counter",
-                                                 llvm::errc::io_error);
-  } else {
-    Count = 42;
-  }
-
-  llvm::SmallVector<int64_t, 4> Result;
-  Result.push_back(Count);
-  return Result;
+CounterGroup::readOrError(StringRef FunctionBytes) const {
+  if (!IsDummyEvent)
+    return EventCounter.readOrError(FunctionBytes);
+  else
+    return SmallVector<int64_t, 1>(1, 42);
 }
 
 int CounterGroup::numValues() const { return 1; }
 #else
 
-void CounterGroup::initRealEvent(const PerfEvent &, pid_t ProcessID) {}
-
-CounterGroup::~CounterGroup() = default;
+void CounterGroup::initRealEvent(pid_t ProcessID) {}
 
 void CounterGroup::start() {}
 
diff --git a/llvm/tools/llvm-exegesis/lib/PerfHelper.h b/llvm/tools/llvm-exegesis/lib/PerfHelper.h
index daf2fbd0e3abc8..e9b73da3836287 100644
--- a/llvm/tools/llvm-exegesis/lib/PerfHelper.h
+++ b/llvm/tools/llvm-exegesis/lib/PerfHelper.h
@@ -77,6 +77,29 @@ class PerfEvent {
   void initRealEvent(StringRef PfmEventString);
 };
 
+// Represents a single event that has been configured in the Linux perf
+// subsystem.
+class ConfiguredEvent {
+public:
+  ConfiguredEvent(PerfEvent &&EventToConfigure);
+
+  void initRealEvent(const pid_t ProcessID);
+  Expected<SmallVector<int64_t>> readOrError(StringRef FunctionBytes) const;
+  int getFileDescriptor() const { return FileDescriptor; }
+  bool isDummyEvent() const {
+    return Event.name() == PerfEvent::DummyEventString;
+  }
+
+  ConfiguredEvent(const ConfiguredEvent &) = delete;
+  ConfiguredEvent(ConfiguredEvent &&other) = default;
+
+  ~ConfiguredEvent();
+
+private:
+  PerfEvent Event;
+  int FileDescriptor = -1;
+};
+
 // Consists of a counter measuring a specific event and associated validation
 // counters measuring execution conditions. All counters in a group are part
 // of a single event group and are thus scheduled on and off the CPU as a single
@@ -89,7 +112,7 @@ class CounterGroup {
   CounterGroup(const CounterGroup &) = delete;
   CounterGroup(CounterGroup &&other) = default;
 
-  virtual ~CounterGroup();
+  virtual ~CounterGroup() = default;
 
   /// Starts the measurement of the event.
   virtual void start();
@@ -108,15 +131,14 @@ class CounterGroup {
 
   virtual int numValues() const;
 
-  int getFileDescriptor() const { return FileDescriptor; }
+  int getFileDescriptor() const { return EventCounter.getFileDescriptor(); }
 
 protected:
-  PerfEvent Event;
-  int FileDescriptor = -1;
+  ConfiguredEvent EventCounter;
   bool IsDummyEvent;
 
 private:
-  void initRealEvent(const PerfEvent &E, pid_t ProcessID);
+  void initRealEvent(pid_t ProcessID);
 };
 
 } // namespace pfm
diff --git a/llvm/tools/llvm-exegesis/lib/X86/X86Counter.cpp b/llvm/tools/llvm-exegesis/lib/X86/X86Counter.cpp
index 55ca2d7146ceb7..96fb0f085a1ee1 100644
--- a/llvm/tools/llvm-exegesis/lib/X86/X86Counter.cpp
+++ b/llvm/tools/llvm-exegesis/lib/X86/X86Counter.cpp
@@ -143,7 +143,7 @@ X86LbrPerfEvent::X86LbrPerfEvent(unsigned SamplingPeriod) {
 X86LbrCounter::X86LbrCounter(pfm::PerfEvent &&NewEvent)
     : CounterGroup(std::move(NewEvent)) {
   MMappedBuffer = mmap(nullptr, kMappedBufferSize, PROT_READ | PROT_WRITE,
-                       MAP_SHARED, FileDescriptor, 0);
+                       MAP_SHARED, getFileDescriptor(), 0);
   if (MMappedBuffer == MAP_FAILED)
     llvm::errs() << "Failed to mmap buffer.";
 }
@@ -154,7 +154,7 @@ X86LbrCounter::~X86LbrCounter() {
 }
 
 void X86LbrCounter::start() {
-  ioctl(FileDescriptor, PERF_EVENT_IOC_REFRESH, 1024 /* kMaxPollsPerFd */);
+  ioctl(getFileDescriptor(), PERF_EVENT_IOC_REFRESH, 1024 /* kMaxPollsPerFd */);
 }
 
 llvm::Error X86LbrCounter::checkLbrSupport() {
@@ -197,7 +197,7 @@ llvm::Error X86LbrCounter::checkLbrSupport() {
 llvm::Expected<llvm::SmallVector<int64_t, 4>>
 X86LbrCounter::readOrError(StringRef FunctionBytes) const {
   // Disable the event before reading
-  ioctl(FileDescriptor, PERF_EVENT_IOC_DISABLE, 0);
+  ioctl(getFileDescriptor(), PERF_EVENT_IOC_DISABLE, 0);
 
   // Find the boundary of the function so that we could filter the LBRs
   // to keep only the relevant records.
@@ -223,7 +223,7 @@ X86LbrCounter::doReadCounter(const void *From, const void *To) const {
   int PollResult = 0;
 
   while (PollResult <= 0) {
-    PollResult = pollLbrPerfEvent(FileDescriptor);
+    PollResult = pollLbrPerfEvent(getFileDescriptor());
     if (PollResult > 0)
       break;
     if (PollResult == -1)

Base automatically changed from users/boomanaiden154/exegesis-validation-counters-countergroup to main January 16, 2024 09:24
This further sets things up for validation events. Having a separate
abstraction for a configured event that is setup as a counter allows for
much easier creation of more events in the future within a single
counter group (like validation counters) without duplicating any code.
@boomanaiden154 boomanaiden154 force-pushed the users/boomanaiden154/exegesis-validation-counters-configuredevent branch from 4300851 to 3741dcd Compare January 16, 2024 09:44
@boomanaiden154 boomanaiden154 merged commit a974303 into main Jan 16, 2024
4 of 6 checks passed
@boomanaiden154 boomanaiden154 deleted the users/boomanaiden154/exegesis-validation-counters-configuredevent branch January 16, 2024 09:58
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…lvm#77900)

This further sets things up for validation events. Having a separate
abstraction for a configured event that is setup as a counter allows for
much easier creation of more events in the future within a single
counter group (like validation counters) without duplicating any code.
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