Re: [PATCH] perf, script: Fix brcntr output with --xed

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Wed, Feb 25, 2026 at 09:28:23AM -0800, Ian Rogers wrote:
> On Tue, Feb 17, 2026 at 5:48 PM Andi Kleen <andi@firstfloor.org> wrote:
> >
> > brcntr in perf script brstack insn currently outputs
> >
> > $ perf record -j any,counter ...
> > $ perf script -F +brcntr,+brstackinsn
> > ...
> >             BC1s 3450809 5665912.127194:     100127
> > cpu_core/cycles/:      7f0475d6cc89 handle_intel.constprop.0+0x2b
> > (/usr/lib64/ld-linux-
> > x86-64.so.2)
> >         intel_check_word.constprop.0+224:
> >         00007f0475d6ca7e        insn: 00 4b db                  br_cntr: # PRED 21 cycles [21]
> > ...
> >
> > This has two issues:
> > - The description says no event is a single dash, but that is not what is printed.
> > - The b in brcntr is ambigious with the hex numbers in insns, which
> >   breaks with --xed. It parses the b as another instruction byte and
> > merges the instruction with a missing b and no space:
> >
> > $ perf script -F +brstackinsn,+brcntr --xed
> > ...
> >         00005618c6d683b5                        jnz 0x5618c6d683bdr_cntr:       # PRED 5 cycles [1396] 8.60 IPC
> >
> > This patches fixes these two problems. It moves the brcntr output into
> > the "#" comment which also looks nicer and also fixes the no event case.
> >
> > $ perf script -F +brstackinsn,+brcntr --xed
> > ...
> >         00005618c6d6624f                        jnz 0x5618c6d65fb7 # br_cntr: -  MISPRED 1 cycles [1398] 3.00 IPC
> >
> > Since the old broken format has shipped for a few releases there is a
> > risk of breaking some existing parser, but since this is a obscure
> > feature I hope they're not too common and can adapt.

:)

> >
> > Signed-off-by: Andi Kleen <andi@firstfloor.org>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> Thanks.
> Ian
> 
> > ---
> >  tools/perf/builtin-script.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 62e43d3c5ad7..ac361aa51094 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -1264,11 +1264,11 @@ static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
> >
> >         if (PRINT_FIELD(BRCNTR)) {
> >                 struct evsel *pos = evsel__leader(evsel);
> > -               unsigned int i = 0, j, num, mask, width;
> > +               unsigned int i = 0, j, num, mask, width, numprinted = 0;
> >
> >                 perf_env__find_br_cntr_info(evsel__env(evsel), NULL, &width);
> >                 mask = (1L << width) - 1;
> > -               printed += fprintf(fp, "br_cntr: ");
> > +               printed += fprintf(fp, "\t# br_cntr: ");
> >                 evlist__for_each_entry_from(evsel->evlist, pos) {
> >                         if (!(pos->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS))
> >                                 continue;
> > @@ -1276,16 +1276,20 @@ static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
> >                                 break;
> >
> >                         num = (br_cntr >> (i++ * width)) & mask;
> > +                       numprinted += num;
> >                         if (!verbose) {
> >                                 for (j = 0; j < num; j++)
> >                                         printed += fprintf(fp, "%s", pos->abbr_name);
> >                         } else
> >                                 printed += fprintf(fp, "%s %d ", pos->name, num);
> >                 }
> > -               printed += fprintf(fp, "\t");
> > +               if (numprinted == 0 && !verbose)
> > +                       fprintf(fp, "-");

I think it should be:       printed += fprintf(fp, "-");

Anyway, it's a nitpick.  I can update it locally.

Thanks,
Namhyung


> > +               printed += fprintf(fp, " ");
> >         }
> >
> > -       printed += fprintf(fp, "#%s%s%s%s",
> > +       printed += fprintf(fp, "%s%s%s%s%s",
> > +                             !PRINT_FIELD(BRCNTR) ? "#" : "",
> >                               en->flags.predicted ? " PRED" : "",
> >                               en->flags.mispred ? " MISPRED" : "",
> >                               en->flags.in_tx ? " INTX" : "",
> > --
> > 2.52.0
> >
> >