The perf_stat_merge_counters() is to aggregate the same events in different
PMUs like in case of uncore or hybrid.  The same logic is in the stat-display
routines but I think it should be handled when it processes the event counters.

As it works on the aggr_counters, it doesn't change the output yet.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-stat.c |  2 +
 tools/perf/util/stat.c    | 96 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/stat.h    |  2 +
 3 files changed, 100 insertions(+)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 49a7e290d778..f90e8f29cb23 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -486,6 +486,8 @@ static void process_counters(void)
 			pr_warning("failed to process counter %s\n", counter->name);
 		counter->err = 0;
 	}
+
+	perf_stat_merge_counters(&stat_config, evsel_list);
 }
 
 static void process_interval(void)
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index 4edfc1c5dc07..1bb197782a34 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -575,6 +575,102 @@ int perf_stat_process_counter(struct perf_stat_config *config,
 	return 0;
 }
 
+static int evsel__merge_aggr_counters(struct evsel *evsel, struct evsel *alias)
+{
+	struct perf_stat_evsel *ps_a = evsel->stats;
+	struct perf_stat_evsel *ps_b = alias->stats;
+	int i;
+
+	if (ps_a->aggr == NULL && ps_b->aggr == NULL)
+		return 0;
+
+	if (ps_a->nr_aggr != ps_b->nr_aggr) {
+		pr_err("Unmatched aggregation mode between aliases\n");
+		return -1;
+	}
+
+	for (i = 0; i < ps_a->nr_aggr; i++) {
+		struct perf_counts_values *aggr_counts_a = &ps_a->aggr[i].counts;
+		struct perf_counts_values *aggr_counts_b = &ps_b->aggr[i].counts;
+
+		/* NB: don't increase aggr.nr for aliases */
+
+		aggr_counts_a->val += aggr_counts_b->val;
+		aggr_counts_a->ena += aggr_counts_b->ena;
+		aggr_counts_a->run += aggr_counts_b->run;
+	}
+
+	return 0;
+}
+/* events should have the same name, scale, unit, cgroup but on different PMUs */
+static bool evsel__is_alias(struct evsel *evsel_a, struct evsel *evsel_b)
+{
+	if (strcmp(evsel__name(evsel_a), evsel__name(evsel_b)))
+		return false;
+
+	if (evsel_a->scale != evsel_b->scale)
+		return false;
+
+	if (evsel_a->cgrp != evsel_b->cgrp)
+		return false;
+
+	if (strcmp(evsel_a->unit, evsel_b->unit))
+		return false;
+
+	if (evsel__is_clock(evsel_a) != evsel__is_clock(evsel_b))
+		return false;
+
+	return !!strcmp(evsel_a->pmu_name, evsel_b->pmu_name);
+}
+
+static void evsel__merge_aliases(struct evsel *evsel)
+{
+	struct evlist *evlist = evsel->evlist;
+	struct evsel *alias;
+
+	alias = list_prepare_entry(evsel, &(evlist->core.entries), core.node);
+	list_for_each_entry_continue(alias, &evlist->core.entries, core.node) {
+		/* Merge the same events on different PMUs. */
+		if (evsel__is_alias(evsel, alias)) {
+			evsel__merge_aggr_counters(evsel, alias);
+			alias->merged_stat = true;
+		}
+	}
+}
+
+static bool evsel__should_merge_hybrid(struct evsel *evsel, struct perf_stat_config *config)
+{
+	struct perf_pmu *pmu;
+
+	if (!config->hybrid_merge)
+		return false;
+
+	pmu = evsel__find_pmu(evsel);
+	return pmu && pmu->is_hybrid;
+}
+
+static void evsel__merge_stats(struct evsel *evsel, struct perf_stat_config *config)
+{
+	/* this evsel is already merged */
+	if (evsel->merged_stat)
+		return;
+
+	if (evsel->auto_merge_stats || evsel__should_merge_hybrid(evsel, config))
+		evsel__merge_aliases(evsel);
+}
+
+/* merge the same uncore and hybrid events if requested */
+void perf_stat_merge_counters(struct perf_stat_config *config, struct evlist *evlist)
+{
+	struct evsel *evsel;
+
+	if (config->no_merge)
+		return;
+
+	evlist__for_each_entry(evlist, evsel)
+		evsel__merge_stats(evsel, config);
+}
+
 int perf_event__process_stat_event(struct perf_session *session,
 				   union perf_event *event)
 {
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index 3a876ad2870b..12cc60ab04e4 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -270,6 +270,8 @@ void evlist__reset_aggr_stats(struct evlist *evlist);
 
 int perf_stat_process_counter(struct perf_stat_config *config,
 			      struct evsel *counter);
+void perf_stat_merge_counters(struct perf_stat_config *config, struct evlist *evlist);
+
 struct perf_tool;
 union perf_event;
 struct perf_session;
-- 
2.38.0.rc1.362.ged0d419d3c-goog

On Sun, Oct 9, 2022 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> The perf_stat_merge_counters() is to aggregate the same events in different
> PMUs like in case of uncore or hybrid.  The same logic is in the stat-display
> routines but I think it should be handled when it processes the event counters.

I think I'm confused as to what a merged counter is. Does it relate to
the evsel leader? How are aliases and merging related?

Thanks,
Ian

>
> As it works on the aggr_counters, it doesn't change the output yet.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-stat.c |  2 +
>  tools/perf/util/stat.c    | 96 +++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/stat.h    |  2 +
>  3 files changed, 100 insertions(+)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 49a7e290d778..f90e8f29cb23 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -486,6 +486,8 @@ static void process_counters(void)
>                         pr_warning("failed to process counter %s\n", counter->name);
>                 counter->err = 0;
>         }
> +
> +       perf_stat_merge_counters(&stat_config, evsel_list);
>  }
>
>  static void process_interval(void)
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index 4edfc1c5dc07..1bb197782a34 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -575,6 +575,102 @@ int perf_stat_process_counter(struct perf_stat_config *config,
>         return 0;
>  }
>
> +static int evsel__merge_aggr_counters(struct evsel *evsel, struct evsel *alias)
> +{
> +       struct perf_stat_evsel *ps_a = evsel->stats;
> +       struct perf_stat_evsel *ps_b = alias->stats;
> +       int i;
> +
> +       if (ps_a->aggr == NULL && ps_b->aggr == NULL)
> +               return 0;
> +
> +       if (ps_a->nr_aggr != ps_b->nr_aggr) {
> +               pr_err("Unmatched aggregation mode between aliases\n");
> +               return -1;
> +       }
> +
> +       for (i = 0; i < ps_a->nr_aggr; i++) {
> +               struct perf_counts_values *aggr_counts_a = &ps_a->aggr[i].counts;
> +               struct perf_counts_values *aggr_counts_b = &ps_b->aggr[i].counts;
> +
> +               /* NB: don't increase aggr.nr for aliases */
> +
> +               aggr_counts_a->val += aggr_counts_b->val;
> +               aggr_counts_a->ena += aggr_counts_b->ena;
> +               aggr_counts_a->run += aggr_counts_b->run;
> +       }
> +
> +       return 0;
> +}
> +/* events should have the same name, scale, unit, cgroup but on different PMUs */
> +static bool evsel__is_alias(struct evsel *evsel_a, struct evsel *evsel_b)
> +{
> +       if (strcmp(evsel__name(evsel_a), evsel__name(evsel_b)))
> +               return false;
> +
> +       if (evsel_a->scale != evsel_b->scale)
> +               return false;
> +
> +       if (evsel_a->cgrp != evsel_b->cgrp)
> +               return false;
> +
> +       if (strcmp(evsel_a->unit, evsel_b->unit))
> +               return false;
> +
> +       if (evsel__is_clock(evsel_a) != evsel__is_clock(evsel_b))
> +               return false;
> +
> +       return !!strcmp(evsel_a->pmu_name, evsel_b->pmu_name);
> +}
> +
> +static void evsel__merge_aliases(struct evsel *evsel)
> +{
> +       struct evlist *evlist = evsel->evlist;
> +       struct evsel *alias;
> +
> +       alias = list_prepare_entry(evsel, &(evlist->core.entries), core.node);
> +       list_for_each_entry_continue(alias, &evlist->core.entries, core.node) {
> +               /* Merge the same events on different PMUs. */
> +               if (evsel__is_alias(evsel, alias)) {
> +                       evsel__merge_aggr_counters(evsel, alias);
> +                       alias->merged_stat = true;
> +               }
> +       }
> +}
> +
> +static bool evsel__should_merge_hybrid(struct evsel *evsel, struct perf_stat_config *config)
> +{
> +       struct perf_pmu *pmu;
> +
> +       if (!config->hybrid_merge)
> +               return false;
> +
> +       pmu = evsel__find_pmu(evsel);
> +       return pmu && pmu->is_hybrid;
> +}
> +
> +static void evsel__merge_stats(struct evsel *evsel, struct perf_stat_config *config)
> +{
> +       /* this evsel is already merged */
> +       if (evsel->merged_stat)
> +               return;
> +
> +       if (evsel->auto_merge_stats || evsel__should_merge_hybrid(evsel, config))
> +               evsel__merge_aliases(evsel);
> +}
> +
> +/* merge the same uncore and hybrid events if requested */
> +void perf_stat_merge_counters(struct perf_stat_config *config, struct evlist *evlist)
> +{
> +       struct evsel *evsel;
> +
> +       if (config->no_merge)
> +               return;
> +
> +       evlist__for_each_entry(evlist, evsel)
> +               evsel__merge_stats(evsel, config);
> +}
> +
>  int perf_event__process_stat_event(struct perf_session *session,
>                                    union perf_event *event)
>  {
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index 3a876ad2870b..12cc60ab04e4 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -270,6 +270,8 @@ void evlist__reset_aggr_stats(struct evlist *evlist);
>
>  int perf_stat_process_counter(struct perf_stat_config *config,
>                               struct evsel *counter);
> +void perf_stat_merge_counters(struct perf_stat_config *config, struct evlist *evlist);
> +
>  struct perf_tool;
>  union perf_event;
>  struct perf_session;
> --
> 2.38.0.rc1.362.ged0d419d3c-goog
>

On Mon, Oct 10, 2022 at 4:31 PM Ian Rogers <irogers@google.com> wrote:
>
> On Sun, Oct 9, 2022 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > The perf_stat_merge_counters() is to aggregate the same events in different
> > PMUs like in case of uncore or hybrid.  The same logic is in the stat-display
> > routines but I think it should be handled when it processes the event counters.
>
> I think I'm confused as to what a merged counter is. Does it relate to
> the evsel leader? How are aliases and merging related?

I'm also not sure if the 'aliases' is a good name but I just followed
the existing name
in collect_data().  It's about uncore and hybrid events afaik, not about groups.

For uncore case, it wants to merge the same event in other uncore together
e.g. uncore_imc_0/cas_count_read/ , uncore_imc_1/cas_count_read/ , ...
to uncore_imc/cas_count_read/ or simply imc/cas_count_read/ .

For hybrid case, it'd merge cpu_core/cycles/ and cpu_atom/cycles/ to
cycles when --hybrid-merge is given.

Thanks,
Namhyung

>
> >
> > As it works on the aggr_counters, it doesn't change the output yet.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/builtin-stat.c |  2 +
> >  tools/perf/util/stat.c    | 96 +++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/stat.h    |  2 +
> >  3 files changed, 100 insertions(+)
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 49a7e290d778..f90e8f29cb23 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -486,6 +486,8 @@ static void process_counters(void)
> >                         pr_warning("failed to process counter %s\n", counter->name);
> >                 counter->err = 0;
> >         }
> > +
> > +       perf_stat_merge_counters(&stat_config, evsel_list);
> >  }
> >
> >  static void process_interval(void)
> > diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> > index 4edfc1c5dc07..1bb197782a34 100644
> > --- a/tools/perf/util/stat.c
> > +++ b/tools/perf/util/stat.c
> > @@ -575,6 +575,102 @@ int perf_stat_process_counter(struct perf_stat_config *config,
> >         return 0;
> >  }
> >
> > +static int evsel__merge_aggr_counters(struct evsel *evsel, struct evsel *alias)
> > +{
> > +       struct perf_stat_evsel *ps_a = evsel->stats;
> > +       struct perf_stat_evsel *ps_b = alias->stats;
> > +       int i;
> > +
> > +       if (ps_a->aggr == NULL && ps_b->aggr == NULL)
> > +               return 0;
> > +
> > +       if (ps_a->nr_aggr != ps_b->nr_aggr) {
> > +               pr_err("Unmatched aggregation mode between aliases\n");
> > +               return -1;
> > +       }
> > +
> > +       for (i = 0; i < ps_a->nr_aggr; i++) {
> > +               struct perf_counts_values *aggr_counts_a = &ps_a->aggr[i].counts;
> > +               struct perf_counts_values *aggr_counts_b = &ps_b->aggr[i].counts;
> > +
> > +               /* NB: don't increase aggr.nr for aliases */
> > +
> > +               aggr_counts_a->val += aggr_counts_b->val;
> > +               aggr_counts_a->ena += aggr_counts_b->ena;
> > +               aggr_counts_a->run += aggr_counts_b->run;
> > +       }
> > +
> > +       return 0;
> > +}
> > +/* events should have the same name, scale, unit, cgroup but on different PMUs */
> > +static bool evsel__is_alias(struct evsel *evsel_a, struct evsel *evsel_b)
> > +{
> > +       if (strcmp(evsel__name(evsel_a), evsel__name(evsel_b)))
> > +               return false;
> > +
> > +       if (evsel_a->scale != evsel_b->scale)
> > +               return false;
> > +
> > +       if (evsel_a->cgrp != evsel_b->cgrp)
> > +               return false;
> > +
> > +       if (strcmp(evsel_a->unit, evsel_b->unit))
> > +               return false;
> > +
> > +       if (evsel__is_clock(evsel_a) != evsel__is_clock(evsel_b))
> > +               return false;
> > +
> > +       return !!strcmp(evsel_a->pmu_name, evsel_b->pmu_name);
> > +}
> > +
> > +static void evsel__merge_aliases(struct evsel *evsel)
> > +{
> > +       struct evlist *evlist = evsel->evlist;
> > +       struct evsel *alias;
> > +
> > +       alias = list_prepare_entry(evsel, &(evlist->core.entries), core.node);
> > +       list_for_each_entry_continue(alias, &evlist->core.entries, core.node) {
> > +               /* Merge the same events on different PMUs. */
> > +               if (evsel__is_alias(evsel, alias)) {
> > +                       evsel__merge_aggr_counters(evsel, alias);
> > +                       alias->merged_stat = true;
> > +               }
> > +       }
> > +}
> > +
> > +static bool evsel__should_merge_hybrid(struct evsel *evsel, struct perf_stat_config *config)
> > +{
> > +       struct perf_pmu *pmu;
> > +
> > +       if (!config->hybrid_merge)
> > +               return false;
> > +
> > +       pmu = evsel__find_pmu(evsel);
> > +       return pmu && pmu->is_hybrid;
> > +}
> > +
> > +static void evsel__merge_stats(struct evsel *evsel, struct perf_stat_config *config)
> > +{
> > +       /* this evsel is already merged */
> > +       if (evsel->merged_stat)
> > +               return;
> > +
> > +       if (evsel->auto_merge_stats || evsel__should_merge_hybrid(evsel, config))
> > +               evsel__merge_aliases(evsel);
> > +}
> > +
> > +/* merge the same uncore and hybrid events if requested */
> > +void perf_stat_merge_counters(struct perf_stat_config *config, struct evlist *evlist)
> > +{
> > +       struct evsel *evsel;
> > +
> > +       if (config->no_merge)
> > +               return;
> > +
> > +       evlist__for_each_entry(evlist, evsel)
> > +               evsel__merge_stats(evsel, config);
> > +}
> > +
> >  int perf_event__process_stat_event(struct perf_session *session,
> >                                    union perf_event *event)
> >  {
> > diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> > index 3a876ad2870b..12cc60ab04e4 100644
> > --- a/tools/perf/util/stat.h
> > +++ b/tools/perf/util/stat.h
> > @@ -270,6 +270,8 @@ void evlist__reset_aggr_stats(struct evlist *evlist);
> >
> >  int perf_stat_process_counter(struct perf_stat_config *config,
> >                               struct evsel *counter);
> > +void perf_stat_merge_counters(struct perf_stat_config *config, struct evlist *evlist);
> > +
> >  struct perf_tool;
> >  union perf_event;
> >  struct perf_session;
> > --
> > 2.38.0.rc1.362.ged0d419d3c-goog
> >