Received-SPF: pass (google.com: domain of linux-kernel+bounces-55776-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240205145819.1943114-1-kan.liang@linux.intel.com>
In-Reply-To: <20240205145819.1943114-1-kan.liang@linux.intel.com>
From: Ian Rogers <irogers@google.com>
Date: Tue, 6 Feb 2024 17:00:49 -0800
Message-ID: <CAP-5=fWB-URhxGG00h9azV-e1tZBWNFpZbzhEW+zbLaQGFeJHQ@mail.gmail.com>
Subject: Re: [PATCH] perf script: Print source line for each jump in brstackinsn
To: kan.liang@linux.intel.com
Cc: acme@kernel.org, namhyung@kernel.org, mingo@kernel.org, 
	peterz@infradead.org, linux-kernel@vger.kernel.org, 
	linux-perf-users@vger.kernel.org, jolsa@kernel.org, adrian.hunter@intel.com, 
	ak@linux.intel.com, amiri.khalil@intel.com, ahmad.yasin@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Feb 5, 2024 at 6:58=E2=80=AFAM <kan.liang@linux.intel.com> wrote:
>
> From: Kan Liang <kan.liang@linux.intel.com>
>
> With the srcline option, the perf script only prints a source line at
> the beginning of a sample with call/ret from functions, but not for
> each jump in brstackinsn. It's useful to print a source line for each
> jump in brstackinsn when the end user analyze the full assembler
> sequences of branch sequences for the sample.
>
> The srccode option can also be used to locate the source code line.
> However, it's printed almost for every line and makes the output less
> readable.
>
>  $perf script -F +brstackinsn,+srcline --xed
>
> Before the patch,
>
>  tchain_edit_deb 1463275 15228549.107820:     282495 instructions:u:     =
       401133 f3+0xd (/home/kan/os.li>
>   tchain_edit.c:22
>         f3+40:  tchain_edit.c:20
>         000000000040114e                        jle 0x401133             =
       # PRED 6 cycles [6]
>         0000000000401133                        movl  -0x4(%rbp), %eax
>         0000000000401136                        and $0x1, %eax
>         0000000000401139                        test %eax, %eax
>         000000000040113b                        jz 0x401143
>         000000000040113d                        addl  $0x1, -0x4(%rbp)
>         0000000000401141                        jmp 0x401147             =
       # PRED 3 cycles [9] 2.00 IPC
>         0000000000401147                        cmpl  $0x3e7, -0x4(%rbp)
>         000000000040114e                        jle 0x401133             =
       # PRED 6 cycles [15] 0.33 IPC
>
> After the patch,
>
>  tchain_edit_deb 1463275 15228549.107820:     282495 instructions:u:     =
       401133 f3+0xd (/home/kan/os.li>
>   tchain_edit.c:22
>         f3+40:  tchain_edit.c:20
>         000000000040114e                        jle 0x401133             =
        srcline: tchain_edit.c:20      # PRED 6 cycles [6]
>         0000000000401133                        movl  -0x4(%rbp), %eax
>         0000000000401136                        and $0x1, %eax
>         0000000000401139                        test %eax, %eax
>         000000000040113b                        jz 0x401143
>         000000000040113d                        addl  $0x1, -0x4(%rbp)
>         0000000000401141                        jmp 0x401147             =
        srcline: tchain_edit.c:23      # PRED 3 cycles [9] 2.00 IPC
>         0000000000401147                        cmpl  $0x3e7, -0x4(%rbp)
>         000000000040114e                        jle 0x401133             =
        srcline: tchain_edit.c:20      # PRED 6 cycles [15] 0.33 IPC
>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-script.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index b1f57401ff23..af63b7c37c8a 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1162,7 +1162,8 @@ static int print_srccode(struct thread *thread, u8 =
cpumode, uint64_t addr)
>  static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
>                             struct perf_insn *x, u8 *inbuf, int len,
>                             int insn, FILE *fp, int *total_cycles,
> -                           struct perf_event_attr *attr)
> +                           struct perf_event_attr *attr,
> +                           struct thread *thread)
>  {
>         int ilen =3D 0;
>         int printed =3D fprintf(fp, "\t%016" PRIx64 "\t%-30s\t", ip,
> @@ -1171,6 +1172,16 @@ static int ip__fprintf_jump(uint64_t ip, struct br=
anch_entry *en,
>         if (PRINT_FIELD(BRSTACKINSNLEN))
>                 printed +=3D fprintf(fp, "ilen: %d\t", ilen);
>
> +       if (PRINT_FIELD(SRCLINE)) {
> +               struct addr_location al;
> +
> +               addr_location__init(&al);
> +               thread__find_map(thread, x->cpumode, ip, &al);
> +               printed +=3D map__fprintf_srcline(al.map, al.addr, " srcl=
ine: ", fp);
> +               printed +=3D fprintf(fp, "\t");
> +               addr_location__exit(&al);
> +       }
> +
>         printed +=3D fprintf(fp, "#%s%s%s%s",
>                               en->flags.predicted ? " PRED" : "",
>                               en->flags.mispred ? " MISPRED" : "",
> @@ -1182,6 +1193,7 @@ static int ip__fprintf_jump(uint64_t ip, struct bra=
nch_entry *en,
>                 if (insn)
>                         printed +=3D fprintf(fp, " %.2f IPC", (float)insn=
 / en->flags.cycles);
>         }
> +
>         return printed + fprintf(fp, "\n");
>  }
>
> @@ -1260,7 +1272,7 @@ static int perf_sample__fprintf_brstackinsn(struct =
perf_sample *sample,
>                                            x.cpumode, x.cpu, &lastsym, at=
tr, fp);
>                 printed +=3D ip__fprintf_jump(entries[nr - 1].from, &entr=
ies[nr - 1],
>                                             &x, buffer, len, 0, fp, &tota=
l_cycles,
> -                                           attr);
> +                                           attr, thread);
>                 if (PRINT_FIELD(SRCCODE))
>                         printed +=3D print_srccode(thread, x.cpumode, ent=
ries[nr - 1].from);
>         }
> @@ -1291,7 +1303,7 @@ static int perf_sample__fprintf_brstackinsn(struct =
perf_sample *sample,
>                         printed +=3D ip__fprintf_sym(ip, thread, x.cpumod=
e, x.cpu, &lastsym, attr, fp);
>                         if (ip =3D=3D end) {
>                                 printed +=3D ip__fprintf_jump(ip, &entrie=
s[i], &x, buffer + off, len - off, ++insn, fp,
> -                                                           &total_cycles=
, attr);
> +                                                           &total_cycles=
, attr, thread);
>                                 if (PRINT_FIELD(SRCCODE))
>                                         printed +=3D print_srccode(thread=
, x.cpumode, ip);
>                                 break;
> --
> 2.35.1
>