* [PATCH v3 0/2] perf report: Implement visual marker for macro fusion in annotate
@ 2017-06-20 12:22 Jin Yao
2017-06-20 12:22 ` [PATCH v3 1/2] perf util: Check for fused instruction Jin Yao
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jin Yao @ 2017-06-20 12:22 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
Macro fusion merges two instructions to a single micro-op. Intel
core platform performs this hardware optimization under limited
circumstances. For example, CMP + JCC can be "fused" and executed
/retired together. While with sampling this can result in the
sample sometimes being on the JCC and sometimes on the CMP.
So for the fused instruction pair, they could be considered
together.
On Nehalem, fused instruction pairs:
cmp/test + jcc.
On other new CPU:
cmp/test/add/sub/and/inc/dec + jcc.
This patch series marks the case clearly by joining the fused
instruction pair in the arrow of the jump.
For example:
│ ┌──cmpl $0x0,argp_program_version_hook
81.93 │ ├──je 20
│ │ lock cmpxchg %esi,0x38a9a4(%rip)
│ │↓ jne 29
│ │↓ jmp 43
11.47 │20:└─→cmpxch %esi,0x38a999(%rip)
Change-log:
-----------
v3: 1. Add checking for Nehalem (CMP, TEST). For other newer
Intel CPUs just check it by default (CMP, TEST, ADD,
SUB, AND, INC, DEC).
2. Use Arnaldo's fix to let the display be better
v2: According to Arnaldo's comments, remove the weak function and
use an arch-specific function instead to check fused instruction
pair.
v1: Inital post
Jin Yao (2):
perf util: Check for fused instruction
perf report: Implement visual marker for macro fusion in annotate
tools/perf/arch/x86/annotate/instructions.c | 37 +++++++++++++++++++++++++++++
tools/perf/ui/browser.c | 29 ++++++++++++++++++++++
tools/perf/ui/browser.h | 2 ++
tools/perf/ui/browsers/annotate.c | 32 +++++++++++++++++++++++++
tools/perf/util/annotate.c | 17 +++++++++++++
tools/perf/util/annotate.h | 3 +++
6 files changed, 120 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] perf util: Check for fused instruction
2017-06-20 12:22 [PATCH v3 0/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
@ 2017-06-20 12:22 ` Jin Yao
2017-07-06 16:24 ` Arnaldo Carvalho de Melo
2017-06-20 12:22 ` [PATCH v3 2/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
2017-07-06 0:42 ` [PATCH v3 0/2] " Jin, Yao
2 siblings, 1 reply; 6+ messages in thread
From: Jin Yao @ 2017-06-20 12:22 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
Macro fusion merges two instructions to a single micro-op. Intel
core platform performs this hardware optimization under limited
circumstances.
For example, CMP + JCC can be "fused" and executed /retired
together. While with sampling this can result in the sample
sometimes being on the JCC and sometimes on the CMP.
So for the fused instruction pair, they could be considered
together.
On Nehalem, fused instruction pairs:
cmp/test + jcc.
On other new CPU:
cmp/test/add/sub/and/inc/dec + jcc.
This patch adds an x86-specific function which checks if 2
instructions are in a "fused" pair. For non-x86 arch, the
function is just NULL.
Change-log:
-----------
v3: Add checking for Nehalem (CMP, TEST). For other newer
Intel CPUs just check it by default (CMP, TEST, ADD,
SUB, AND, INC, DEC).
v2: Remove the origial weak function. Arnaldo points out
that doing it as a weak function that will be overridden
by the host arch doesn't work. So now it's implemented
as an arch-specific function.
v1: Initial post
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/arch/x86/annotate/instructions.c | 37 +++++++++++++++++++++++++++++
tools/perf/util/annotate.c | 12 ++++++++++
tools/perf/util/annotate.h | 2 ++
3 files changed, 51 insertions(+)
diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index c1625f2..8d06dd4 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -76,3 +76,40 @@ static struct ins x86__instructions[] = {
{ .name = "xbeginq", .ops = &jump_ops, },
{ .name = "retq", .ops = &ret_ops, },
};
+
+static bool x86__ins_is_fused(char *cpuid, const char *ins1, const char *ins2)
+{
+ unsigned int family, model, stepping;
+ int ret;
+
+ /*
+ * cpuid = "GenuineIntel,family,model,stepping"
+ */
+ ret = sscanf(cpuid, "%*[^,],%u,%u,%u", &family, &model, &stepping);
+
+ if ((ret != 3) || (family != 6) || (model < 0x1e) ||
+ strstr(ins2, "jmp")) {
+ return false;
+ }
+
+ if (model == 0x1e) {
+ /* Nehalem */
+ if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
+ strstr(ins1, "test")) {
+ return true;
+ }
+ } else {
+ /* Newer platform */
+ if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
+ strstr(ins1, "test") ||
+ strstr(ins1, "add") ||
+ strstr(ins1, "sub") ||
+ strstr(ins1, "and") ||
+ strstr(ins1, "inc") ||
+ strstr(ins1, "dec")) {
+ return true;
+ }
+ }
+
+ return false;
+}
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index be1caab..8cf6025 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -48,6 +48,8 @@ struct arch {
bool initialized;
void *priv;
int (*init)(struct arch *arch);
+ bool (*ins_is_fused)(char *cpuid, const char *ins1,
+ const char *ins2);
struct {
char comment_char;
char skip_functions_char;
@@ -129,6 +131,7 @@ static struct arch architectures[] = {
.name = "x86",
.instructions = x86__instructions,
.nr_instructions = ARRAY_SIZE(x86__instructions),
+ .ins_is_fused = x86__ins_is_fused,
.objdump = {
.comment_char = '#',
},
@@ -171,6 +174,15 @@ int ins__scnprintf(struct ins *ins, char *bf, size_t size,
return ins__raw_scnprintf(ins, bf, size, ops);
}
+bool ins__is_fused(struct arch *arch, char *cpuid, const char *ins1,
+ const char *ins2)
+{
+ if (!arch || !arch->ins_is_fused || !cpuid)
+ return false;
+
+ return arch->ins_is_fused(cpuid, ins1, ins2);
+}
+
static int call__parse(struct arch *arch, struct ins_operands *ops, struct map *map)
{
char *endptr, *tok, *name;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 2105503..35cb7b5 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -53,6 +53,8 @@ bool ins__is_jump(const struct ins *ins);
bool ins__is_call(const struct ins *ins);
bool ins__is_ret(const struct ins *ins);
int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops);
+bool ins__is_fused(struct arch *arch, char *cpuid, const char *ins1,
+ const char *ins2);
struct annotation;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] perf report: Implement visual marker for macro fusion in annotate
2017-06-20 12:22 [PATCH v3 0/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
2017-06-20 12:22 ` [PATCH v3 1/2] perf util: Check for fused instruction Jin Yao
@ 2017-06-20 12:22 ` Jin Yao
2017-07-06 0:42 ` [PATCH v3 0/2] " Jin, Yao
2 siblings, 0 replies; 6+ messages in thread
From: Jin Yao @ 2017-06-20 12:22 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
For marking the fused instructions clearly, This patch adds a
line before the first instruction of pair and joins it with the
arrow of the jump.
For example, when je is selected in annotate view, the line
before cmpl is displayed and joins the arrow of je.
│ ┌──cmpl $0x0,argp_program_version_hook
81.93 │ ├──je 20
│ │ lock cmpxchg %esi,0x38a9a4(%rip)
│ │↓ jne 29
│ │↓ jmp 43
11.47 │20:└─→cmpxch %esi,0x38a999(%rip)
That means the cmpl+je is fused instruction pair and they should
be considered together.
Change-log:
-----------
v3: Use Arnaldo's fix to let the display be better.
To get the evsel->evlist->env->cpuid, save the evsel in
annotate_browser.
v2: No more changes, just uses a new function "ins__is_fused"
to check if the instructions are fused.
v1: Initial post
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/ui/browser.c | 29 +++++++++++++++++++++++++++++
tools/perf/ui/browser.h | 2 ++
tools/perf/ui/browsers/annotate.c | 32 ++++++++++++++++++++++++++++++++
tools/perf/util/annotate.c | 5 +++++
tools/perf/util/annotate.h | 1 +
5 files changed, 69 insertions(+)
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index a4d3762..9ef7677 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -738,6 +738,35 @@ void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
__ui_browser__line_arrow_down(browser, column, start, end);
}
+void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
+ unsigned int row, bool arrow_down)
+{
+ unsigned int end_row;
+
+ if (row >= browser->top_idx)
+ end_row = row - browser->top_idx;
+ else
+ return;
+
+ SLsmg_set_char_set(1);
+
+ if (arrow_down) {
+ ui_browser__gotorc(browser, end_row, column - 1);
+ SLsmg_write_char(SLSMG_ULCORN_CHAR);
+ ui_browser__gotorc(browser, end_row, column);
+ SLsmg_draw_hline(2);
+ ui_browser__gotorc(browser, end_row + 1, column - 1);
+ SLsmg_write_char(SLSMG_LTEE_CHAR);
+ } else {
+ ui_browser__gotorc(browser, end_row, column - 1);
+ SLsmg_write_char(SLSMG_LTEE_CHAR);
+ ui_browser__gotorc(browser, end_row, column);
+ SLsmg_draw_hline(2);
+ }
+
+ SLsmg_set_char_set(0);
+}
+
void ui_browser__init(void)
{
int i = 0;
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index be3b70e..a12eff7 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -43,6 +43,8 @@ void ui_browser__printf(struct ui_browser *browser, const char *fmt, ...);
void ui_browser__write_graph(struct ui_browser *browser, int graph);
void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
u64 start, u64 end);
+void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
+ unsigned int row, bool arrow_down);
void __ui_browser__show_title(struct ui_browser *browser, const char *title);
void ui_browser__show_title(struct ui_browser *browser, const char *title);
int ui_browser__show(struct ui_browser *browser, const char *title,
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 27f41f2..d3c7109 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -9,6 +9,7 @@
#include "../../util/symbol.h"
#include "../../util/evsel.h"
#include "../../util/config.h"
+#include "../../util/evlist.h"
#include <inttypes.h>
#include <pthread.h>
#include <linux/kernel.h>
@@ -55,6 +56,7 @@ struct annotate_browser {
struct disasm_line *selection;
struct disasm_line **offsets;
struct arch *arch;
+ struct perf_evsel *evsel;
int nr_events;
u64 start;
int nr_asm_entries;
@@ -272,6 +274,28 @@ static bool disasm_line__is_valid_jump(struct disasm_line *dl, struct symbol *sy
return true;
}
+static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
+{
+ struct disasm_line *pos = list_prev_entry(cursor, node);
+ struct perf_evsel *evsel = ab->evsel;
+ const char *name;
+
+ if (!pos || !evsel || !evsel->evlist || !evsel->evlist->env ||
+ !evsel->evlist->env->cpuid)
+ return false;
+
+ if (ins__is_lock(&pos->ins))
+ name = pos->ops.locked.ins.name;
+ else
+ name = pos->ins.name;
+
+ if (!name || !cursor->ins.name)
+ return false;
+
+ return ins__is_fused(ab->arch, evsel->evlist->env->cpuid, name,
+ cursor->ins.name);
+}
+
static void annotate_browser__draw_current_jump(struct ui_browser *browser)
{
struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
@@ -307,6 +331,13 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
ui_browser__set_color(browser, HE_COLORSET_JUMP_ARROWS);
__ui_browser__line_arrow(browser, pcnt_width + 2 + ab->addr_width,
from, to);
+
+ if (is_fused(ab, cursor)) {
+ ui_browser__mark_fused(browser,
+ pcnt_width + 3 + ab->addr_width,
+ from - 1,
+ to > from ? true : false);
+ }
}
static unsigned int annotate_browser__refresh(struct ui_browser *browser)
@@ -1120,6 +1151,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
browser.b.nr_entries = browser.nr_entries;
browser.b.entries = ¬es->src->source,
browser.b.width += 18; /* Percentage */
+ browser.evsel = evsel;
if (annotate_browser__opts.hide_src_code)
annotate_browser__init_asm_mode(&browser);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 8cf6025..0688837 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -514,6 +514,11 @@ bool ins__is_ret(const struct ins *ins)
return ins->ops == &ret_ops;
}
+bool ins__is_lock(const struct ins *ins)
+{
+ return ins->ops == &lock_ops;
+}
+
static int ins__key_cmp(const void *name, const void *insp)
{
const struct ins *ins = insp;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 35cb7b5..2f5735e 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -52,6 +52,7 @@ struct ins_ops {
bool ins__is_jump(const struct ins *ins);
bool ins__is_call(const struct ins *ins);
bool ins__is_ret(const struct ins *ins);
+bool ins__is_lock(const struct ins *ins);
int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops);
bool ins__is_fused(struct arch *arch, char *cpuid, const char *ins1,
const char *ins2);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] perf report: Implement visual marker for macro fusion in annotate
2017-06-20 12:22 [PATCH v3 0/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
2017-06-20 12:22 ` [PATCH v3 1/2] perf util: Check for fused instruction Jin Yao
2017-06-20 12:22 ` [PATCH v3 2/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
@ 2017-07-06 0:42 ` Jin, Yao
2 siblings, 0 replies; 6+ messages in thread
From: Jin, Yao @ 2017-07-06 0:42 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin
Hi Arnaldo,
Is this series OK?
Thanks
Jin Yao
On 6/20/2017 8:22 PM, Jin Yao wrote:
> Macro fusion merges two instructions to a single micro-op. Intel
> core platform performs this hardware optimization under limited
> circumstances. For example, CMP + JCC can be "fused" and executed
> /retired together. While with sampling this can result in the
> sample sometimes being on the JCC and sometimes on the CMP.
> So for the fused instruction pair, they could be considered
> together.
>
> On Nehalem, fused instruction pairs:
> cmp/test + jcc.
>
> On other new CPU:
> cmp/test/add/sub/and/inc/dec + jcc.
>
> This patch series marks the case clearly by joining the fused
> instruction pair in the arrow of the jump.
>
> For example:
>
> │ ┌──cmpl $0x0,argp_program_version_hook
> 81.93 │ ├──je 20
> │ │ lock cmpxchg %esi,0x38a9a4(%rip)
> │ │↓ jne 29
> │ │↓ jmp 43
> 11.47 │20:└─→cmpxch %esi,0x38a999(%rip)
>
> Change-log:
> -----------
> v3: 1. Add checking for Nehalem (CMP, TEST). For other newer
> Intel CPUs just check it by default (CMP, TEST, ADD,
> SUB, AND, INC, DEC).
>
> 2. Use Arnaldo's fix to let the display be better
>
> v2: According to Arnaldo's comments, remove the weak function and
> use an arch-specific function instead to check fused instruction
> pair.
>
> v1: Inital post
>
> Jin Yao (2):
> perf util: Check for fused instruction
> perf report: Implement visual marker for macro fusion in annotate
>
> tools/perf/arch/x86/annotate/instructions.c | 37 +++++++++++++++++++++++++++++
> tools/perf/ui/browser.c | 29 ++++++++++++++++++++++
> tools/perf/ui/browser.h | 2 ++
> tools/perf/ui/browsers/annotate.c | 32 +++++++++++++++++++++++++
> tools/perf/util/annotate.c | 17 +++++++++++++
> tools/perf/util/annotate.h | 3 +++
> 6 files changed, 120 insertions(+)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] perf util: Check for fused instruction
2017-06-20 12:22 ` [PATCH v3 1/2] perf util: Check for fused instruction Jin Yao
@ 2017-07-06 16:24 ` Arnaldo Carvalho de Melo
2017-07-07 1:09 ` Jin, Yao
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-07-06 16:24 UTC (permalink / raw)
To: Jin Yao
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
Em Tue, Jun 20, 2017 at 08:22:09PM +0800, Jin Yao escreveu:
> Macro fusion merges two instructions to a single micro-op. Intel
> core platform performs this hardware optimization under limited
> circumstances.
>
> For example, CMP + JCC can be "fused" and executed /retired
> together. While with sampling this can result in the sample
> sometimes being on the JCC and sometimes on the CMP.
> So for the fused instruction pair, they could be considered
> together.
>
> On Nehalem, fused instruction pairs:
> cmp/test + jcc.
>
> On other new CPU:
> cmp/test/add/sub/and/inc/dec + jcc.
>
> This patch adds an x86-specific function which checks if 2
> instructions are in a "fused" pair. For non-x86 arch, the
> function is just NULL.
>
> Change-log:
> -----------
> v3: Add checking for Nehalem (CMP, TEST). For other newer
> Intel CPUs just check it by default (CMP, TEST, ADD,
> SUB, AND, INC, DEC).
>
> v2: Remove the origial weak function. Arnaldo points out
> that doing it as a weak function that will be overridden
> by the host arch doesn't work. So now it's implemented
> as an arch-specific function.
>
> v1: Initial post
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/arch/x86/annotate/instructions.c | 37 +++++++++++++++++++++++++++++
> tools/perf/util/annotate.c | 12 ++++++++++
> tools/perf/util/annotate.h | 2 ++
> 3 files changed, 51 insertions(+)
>
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index c1625f2..8d06dd4 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -76,3 +76,40 @@ static struct ins x86__instructions[] = {
> { .name = "xbeginq", .ops = &jump_ops, },
> { .name = "retq", .ops = &ret_ops, },
> };
> +
> +static bool x86__ins_is_fused(char *cpuid, const char *ins1, const char *ins2)
> +{
> + unsigned int family, model, stepping;
> + int ret;
> +
> + /*
> + * cpuid = "GenuineIntel,family,model,stepping"
> + */
> + ret = sscanf(cpuid, "%*[^,],%u,%u,%u", &family, &model, &stepping);
So, looking at the next patch, that uses this, I see that you'll call
this everytime that jump arrow will be printed, why not do this when
doing the initial disassembly, and having this info cached in the struct
ins or disasm_line (haven't looked closely to provide exact instructions
on how to do it)?
Even more, you could do this model checking just once per disassembly,
then use it as you go reading the disassembly lines, marking them as
fused/not fused and then at jump arror printing just look at a flag, no?
- Arnaldo
> + if ((ret != 3) || (family != 6) || (model < 0x1e) ||
> + strstr(ins2, "jmp")) {
> + return false;
> + }
> +
> + if (model == 0x1e) {
> + /* Nehalem */
> + if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
> + strstr(ins1, "test")) {
> + return true;
> + }
> + } else {
> + /* Newer platform */
> + if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
> + strstr(ins1, "test") ||
> + strstr(ins1, "add") ||
> + strstr(ins1, "sub") ||
> + strstr(ins1, "and") ||
> + strstr(ins1, "inc") ||
> + strstr(ins1, "dec")) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index be1caab..8cf6025 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -48,6 +48,8 @@ struct arch {
> bool initialized;
> void *priv;
> int (*init)(struct arch *arch);
> + bool (*ins_is_fused)(char *cpuid, const char *ins1,
> + const char *ins2);
> struct {
> char comment_char;
> char skip_functions_char;
> @@ -129,6 +131,7 @@ static struct arch architectures[] = {
> .name = "x86",
> .instructions = x86__instructions,
> .nr_instructions = ARRAY_SIZE(x86__instructions),
> + .ins_is_fused = x86__ins_is_fused,
> .objdump = {
> .comment_char = '#',
> },
> @@ -171,6 +174,15 @@ int ins__scnprintf(struct ins *ins, char *bf, size_t size,
> return ins__raw_scnprintf(ins, bf, size, ops);
> }
>
> +bool ins__is_fused(struct arch *arch, char *cpuid, const char *ins1,
> + const char *ins2)
> +{
> + if (!arch || !arch->ins_is_fused || !cpuid)
> + return false;
> +
> + return arch->ins_is_fused(cpuid, ins1, ins2);
> +}
> +
> static int call__parse(struct arch *arch, struct ins_operands *ops, struct map *map)
> {
> char *endptr, *tok, *name;
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 2105503..35cb7b5 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -53,6 +53,8 @@ bool ins__is_jump(const struct ins *ins);
> bool ins__is_call(const struct ins *ins);
> bool ins__is_ret(const struct ins *ins);
> int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops);
> +bool ins__is_fused(struct arch *arch, char *cpuid, const char *ins1,
> + const char *ins2);
>
> struct annotation;
>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] perf util: Check for fused instruction
2017-07-06 16:24 ` Arnaldo Carvalho de Melo
@ 2017-07-07 1:09 ` Jin, Yao
0 siblings, 0 replies; 6+ messages in thread
From: Jin, Yao @ 2017-07-07 1:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
>> +
>> +static bool x86__ins_is_fused(char *cpuid, const char *ins1, const char *ins2)
>> +{
>> + unsigned int family, model, stepping;
>> + int ret;
>> +
>> + /*
>> + * cpuid = "GenuineIntel,family,model,stepping"
>> + */
>> + ret = sscanf(cpuid, "%*[^,],%u,%u,%u", &family, &model, &stepping);
> So, looking at the next patch, that uses this, I see that you'll call
> this everytime that jump arrow will be printed, why not do this when
> doing the initial disassembly, and having this info cached in the struct
> ins or disasm_line (haven't looked closely to provide exact instructions
> on how to do it)?
Yes, you're right. It doesn't need to do this model checking each time.
I will move the model checking to other place and record the model in
annotate_browser.
> Even more, you could do this model checking just once per disassembly,
> then use it as you go reading the disassembly lines, marking them as
> fused/not fused and then at jump arror printing just look at a flag, no?
>
> - Arnaldo
>
For this, I have another consideration.
The fused instruction pair consists of 2 instructions. One is jump, the
other is "CMP/TEST/...".
If I check the fused instruction pair once per disassembly, I need to
check if one instruction is jump and the other is "CMP/TEST/..." for all
instructions.
While in annotate_browser__draw_current_jump(), it has already checked
if a disasm_line is a valid jump, so the jump checking will be duplicated.
The current implementation is just performing the fused checking when
user moves the cursor on the jump instruction. It doesn't need to add
additional jump checking.
So the current way may be acceptable as well?
Thanks
Jin Yao
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-07 1:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-20 12:22 [PATCH v3 0/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
2017-06-20 12:22 ` [PATCH v3 1/2] perf util: Check for fused instruction Jin Yao
2017-07-06 16:24 ` Arnaldo Carvalho de Melo
2017-07-07 1:09 ` Jin, Yao
2017-06-20 12:22 ` [PATCH v3 2/2] perf report: Implement visual marker for macro fusion in annotate Jin Yao
2017-07-06 0:42 ` [PATCH v3 0/2] " 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.