* [PATCH] perf report: Fix wrong jump arrow
@ 2018-01-29 10:57 Jin Yao
2018-02-09 15:15 ` Arnaldo Carvalho de Melo
2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jin Yao
0 siblings, 2 replies; 4+ messages in thread
From: Jin Yao @ 2018-01-29 10:57 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
When we use perf report interactive annotate view, we can see
the position of jump arrow is not correct. For example,
1. perf record -b ...
2. perf report
3. In interactive mode, select Annotate 'function'
Percent│ IPC Cycle
│ if (flag)
1.37 │0.4┌── 1 ↓ je 82
│ │ x += x / y + y / x;
0.00 │0.4│ 1310 movsd (%rsp),%xmm0
0.00 │0.4│ 565 movsd 0x8(%rsp),%xmm4
│0.4│ movsd 0x8(%rsp),%xmm1
│0.4│ movsd (%rsp),%xmm3
│0.4│ divsd %xmm4,%xmm0
0.00 │0.4│ 579 divsd %xmm3,%xmm1
│0.4│ movsd (%rsp),%xmm2
│0.4│ addsd %xmm1,%xmm0
│0.4│ addsd %xmm2,%xmm0
0.00 │0.4│ movsd %xmm0,(%rsp)
│ │ volatile double x = 1212121212, y = 121212;
│ │
│ │ s_randseed = time(0);
│ │ srand(s_randseed);
│ │
│ │ for (i = 0; i < 2000000000; i++) {
1.37 │0.4└─→ 82: sub $0x1,%ebx
28.21 │0.48 17 ↑ jne 38
The jump arrow in above example is not correct. It should add the
width of IPC and Cycle.
With this patch, the result is:
Percent│ IPC Cycle
│ if (flag)
1.37 │0.48 1 ┌──je 82
│ │ x += x / y + y / x;
0.00 │0.48 1310 │ movsd (%rsp),%xmm0
0.00 │0.48 565 │ movsd 0x8(%rsp),%xmm4
│0.48 │ movsd 0x8(%rsp),%xmm1
│0.48 │ movsd (%rsp),%xmm3
│0.48 │ divsd %xmm4,%xmm0
0.00 │0.48 579 │ divsd %xmm3,%xmm1
│0.48 │ movsd (%rsp),%xmm2
│0.48 │ addsd %xmm1,%xmm0
│0.48 │ addsd %xmm2,%xmm0
0.00 │0.48 │ movsd %xmm0,(%rsp)
│ │ volatile double x = 1212121212, y = 121212;
│ │
│ │ s_randseed = time(0);
│ │ srand(s_randseed);
│ │
│ │ for (i = 0; i < 2000000000; i++) {
1.37 │0.48 82:└─→sub $0x1,%ebx
28.21 │0.48 17 ↑ jne 38
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/ui/browsers/annotate.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 2864279..e2f6663 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -319,6 +319,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
struct map_symbol *ms = ab->b.priv;
struct symbol *sym = ms->sym;
u8 pcnt_width = annotate_browser__pcnt_width(ab);
+ int width = 0;
/* PLT symbols contain external offsets */
if (strstr(sym->name, "@plt"))
@@ -340,13 +341,17 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
to = (u64)btarget->idx;
}
+ if (ab->have_cycles)
+ width = IPC_WIDTH + CYCLES_WIDTH;
+
ui_browser__set_color(browser, HE_COLORSET_JUMP_ARROWS);
- __ui_browser__line_arrow(browser, pcnt_width + 2 + ab->addr_width,
+ __ui_browser__line_arrow(browser,
+ pcnt_width + 2 + ab->addr_width + width,
from, to);
if (is_fused(ab, cursor)) {
ui_browser__mark_fused(browser,
- pcnt_width + 3 + ab->addr_width,
+ pcnt_width + 3 + ab->addr_width + width,
from - 1,
to > from ? true : false);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf report: Fix wrong jump arrow
2018-01-29 10:57 [PATCH] perf report: Fix wrong jump arrow Jin Yao
@ 2018-02-09 15:15 ` Arnaldo Carvalho de Melo
2018-02-12 12:24 ` Jin, Yao
2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jin Yao
1 sibling, 1 reply; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-02-09 15:15 UTC (permalink / raw)
To: Jin Yao
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
Em Mon, Jan 29, 2018 at 06:57:53PM +0800, Jin Yao escreveu:
> When we use perf report interactive annotate view, we can see
> the position of jump arrow is not correct. For example,
>
> 1. perf record -b ...
> 2. perf report
> 3. In interactive mode, select Annotate 'function'
>
> Percent│ IPC Cycle
> │ if (flag)
> 1.37 │0.4┌── 1 ↓ je 82
> │ │ x += x / y + y / x;
Applied and added this to the cset log, please check:
Committer notes:
Please note that only from LBRv5 (according to Jiri) onwards, i.e. >=
Skylake is that we'll have the cycles counts in each branch record
entry, so to see the Cycles and IPC columns, and be able to test this
patch, one need a capable hardware.
While applying this I first tested it on a Broadwell class machine and
couldn't get those columns, will add code to the annotate browser to
warn the user about that, i.e. you have branch records, but no cycles,
use a more recent hardware to get the cycles and IPC columns.
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf report: Fix wrong jump arrow
2018-02-09 15:15 ` Arnaldo Carvalho de Melo
@ 2018-02-12 12:24 ` Jin, Yao
0 siblings, 0 replies; 4+ messages in thread
From: Jin, Yao @ 2018-02-12 12:24 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
Hi Arnaldo,
Thanks for applying the patch. Yes the issue only happens on skl+.
The committer notes are very good and clear.
Thanks
Jin Yao
On 2/9/2018 11:15 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jan 29, 2018 at 06:57:53PM +0800, Jin Yao escreveu:
>> When we use perf report interactive annotate view, we can see
>> the position of jump arrow is not correct. For example,
>>
>> 1. perf record -b ...
>> 2. perf report
>> 3. In interactive mode, select Annotate 'function'
>>
>> Percent│ IPC Cycle
>> │ if (flag)
>> 1.37 │0.4┌── 1 ↓ je 82
>> │ │ x += x / y + y / x;
>
> Applied and added this to the cset log, please check:
>
> Committer notes:
>
> Please note that only from LBRv5 (according to Jiri) onwards, i.e. >=
> Skylake is that we'll have the cycles counts in each branch record
> entry, so to see the Cycles and IPC columns, and be able to test this
> patch, one need a capable hardware.
>
> While applying this I first tested it on a Broadwell class machine and
> couldn't get those columns, will add code to the annotate browser to
> warn the user about that, i.e. you have branch records, but no cycles,
> use a more recent hardware to get the cycles and IPC columns.
>
> - Arnaldo
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:perf/core] perf report: Fix wrong jump arrow
2018-01-29 10:57 [PATCH] perf report: Fix wrong jump arrow Jin Yao
2018-02-09 15:15 ` Arnaldo Carvalho de Melo
@ 2018-02-17 11:34 ` tip-bot for Jin Yao
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Jin Yao @ 2018-02-17 11:34 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, tglx, mingo, ak, alexander.shishkin, yao.jin, acme,
kan.liang, peterz, linux-kernel, jolsa, yao.jin
Commit-ID: b40982e8468b46b8f7f5bba5a7e541ec04a29d7d
Gitweb: https://git.kernel.org/tip/b40982e8468b46b8f7f5bba5a7e541ec04a29d7d
Author: Jin Yao <yao.jin@linux.intel.com>
AuthorDate: Mon, 29 Jan 2018 18:57:53 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 16 Feb 2018 14:55:47 -0300
perf report: Fix wrong jump arrow
When we use perf report interactive annotate view, we can see
the position of jump arrow is not correct. For example,
1. perf record -b ...
2. perf report
3. In interactive mode, select Annotate 'function'
Percent│ IPC Cycle
│ if (flag)
1.37 │0.4┌── 1 ↓ je 82
│ │ x += x / y + y / x;
0.00 │0.4│ 1310 movsd (%rsp),%xmm0
0.00 │0.4│ 565 movsd 0x8(%rsp),%xmm4
│0.4│ movsd 0x8(%rsp),%xmm1
│0.4│ movsd (%rsp),%xmm3
│0.4│ divsd %xmm4,%xmm0
0.00 │0.4│ 579 divsd %xmm3,%xmm1
│0.4│ movsd (%rsp),%xmm2
│0.4│ addsd %xmm1,%xmm0
│0.4│ addsd %xmm2,%xmm0
0.00 │0.4│ movsd %xmm0,(%rsp)
│ │ volatile double x = 1212121212, y = 121212;
│ │
│ │ s_randseed = time(0);
│ │ srand(s_randseed);
│ │
│ │ for (i = 0; i < 2000000000; i++) {
1.37 │0.4└─→ 82: sub $0x1,%ebx
28.21 │0.48 17 ↑ jne 38
The jump arrow in above example is not correct. It should add the
width of IPC and Cycle.
With this patch, the result is:
Percent│ IPC Cycle
│ if (flag)
1.37 │0.48 1 ┌──je 82
│ │ x += x / y + y / x;
0.00 │0.48 1310 │ movsd (%rsp),%xmm0
0.00 │0.48 565 │ movsd 0x8(%rsp),%xmm4
│0.48 │ movsd 0x8(%rsp),%xmm1
│0.48 │ movsd (%rsp),%xmm3
│0.48 │ divsd %xmm4,%xmm0
0.00 │0.48 579 │ divsd %xmm3,%xmm1
│0.48 │ movsd (%rsp),%xmm2
│0.48 │ addsd %xmm1,%xmm0
│0.48 │ addsd %xmm2,%xmm0
0.00 │0.48 │ movsd %xmm0,(%rsp)
│ │ volatile double x = 1212121212, y = 121212;
│ │
│ │ s_randseed = time(0);
│ │ srand(s_randseed);
│ │
│ │ for (i = 0; i < 2000000000; i++) {
1.37 │0.48 82:└─→sub $0x1,%ebx
28.21 │0.48 17 ↑ jne 38
Committer notes:
Please note that only from LBRv5 (according to Jiri) onwards, i.e. >=
Skylake is that we'll have the cycles counts in each branch record
entry, so to see the Cycles and IPC columns, and be able to test this
patch, one need a capable hardware.
While applying this I first tested it on a Broadwell class machine and
couldn't get those columns, will add code to the annotate browser to
warn the user about that, i.e. you have branch records, but no cycles,
use a more recent hardware to get the cycles and IPC columns.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1517223473-14750-1-git-send-email-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/annotate.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 2864279..e2f6663 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -319,6 +319,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
struct map_symbol *ms = ab->b.priv;
struct symbol *sym = ms->sym;
u8 pcnt_width = annotate_browser__pcnt_width(ab);
+ int width = 0;
/* PLT symbols contain external offsets */
if (strstr(sym->name, "@plt"))
@@ -340,13 +341,17 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
to = (u64)btarget->idx;
}
+ if (ab->have_cycles)
+ width = IPC_WIDTH + CYCLES_WIDTH;
+
ui_browser__set_color(browser, HE_COLORSET_JUMP_ARROWS);
- __ui_browser__line_arrow(browser, pcnt_width + 2 + ab->addr_width,
+ __ui_browser__line_arrow(browser,
+ pcnt_width + 2 + ab->addr_width + width,
from, to);
if (is_fused(ab, cursor)) {
ui_browser__mark_fused(browser,
- pcnt_width + 3 + ab->addr_width,
+ pcnt_width + 3 + ab->addr_width + width,
from - 1,
to > from ? true : false);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-17 11:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-29 10:57 [PATCH] perf report: Fix wrong jump arrow Jin Yao
2018-02-09 15:15 ` Arnaldo Carvalho de Melo
2018-02-12 12:24 ` Jin, Yao
2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jin Yao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.