[PATCH] D48576: [llvm-mca] Register listeners with stages; remove Pipeline dependency from Stage.

Clement Courbet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 25 23:36:43 PDT 2018


courbet added inline comments.


================
Comment at: tools/llvm-mca/DispatchStage.cpp:36
+
+void DispatchStage::notifyStallEvent(const HWStallEvent &Event) {
+  for (HWEventListener *Listener : Listeners)
----------------
This pattern appears quite a lot in various stages.  One solution to avoid duplication would be to change the `virtual void onXXXEvent(constHWXXXEvent&)` callback to `virtual void onEvent(const HWXXXEvent&)`. Then there can be a single `notifyEvent` function in `Stage`:

```
template <typename EventT>
void Stage::notifyEvent(const EventT &Event) const {
  for (HWEventListener *Listener : Listeners)
    Listener->onEvent(Event);
}
```

What do you think ?




================
Comment at: tools/llvm-mca/Stage.h:31
+protected:
+  std::set<HWEventListener *> Listeners;
+
----------------
I think this should be made private and only exposed to subclasses as a const accessor:

```
protected:
  const std::set& getListeners() const { return Listeners; }
private:
  std::set<HWEventListener *> Listeners;
```



https://reviews.llvm.org/D48576





More information about the llvm-commits mailing list