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

[Clement Courbet via Phabricator via llvm-commits]

courbet accepted this revision.
courbet added inline comments.
This revision is now accepted and ready to land.


================
Comment at: tools/llvm-mca/DispatchStage.cpp:36
+
+void DispatchStage::notifyStallEvent(const HWStallEvent &Event) {
+  for (HWEventListener *Listener : Listeners)
----------------
mattd wrote:
> courbet wrote:
> > 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 ?
> > 
> > 
> I like the idea. However, it feels a bit more of a functional change than what the current  patch is trying to achieve.  I'd be happy to make this suggested change in separate patch.  But if you feel otherwise, I can make the appropriate changes to this patch.
Sounds good to me.


https://reviews.llvm.org/D48576