[PATCH v4 0/2] perf report: Implement visual marker for macro fusion in annotate

[PATCH v4 0/2] perf report: Implement visual marker for macro fusion in annotate

[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:
-----------
v4: Move the CPU model checking to symbol__disassemble and save
    the family/model in arch structure. It avoids checking everytime
    when display the jump arrows.

    The patch set is still performing the fused checking when user
    moves the cursor on the jump instruction.

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 | 46 +++++++++++++++++++++++++++++
 tools/perf/builtin-top.c                    |  2 +-
 tools/perf/ui/browser.c                     | 29 ++++++++++++++++++
 tools/perf/ui/browser.h                     |  2 ++
 tools/perf/ui/browsers/annotate.c           | 30 ++++++++++++++++++-
 tools/perf/ui/gtk/annotate.c                |  2 +-
 tools/perf/util/annotate.c                  | 27 +++++++++++++++--
 tools/perf/util/annotate.h                  |  4 ++-
 8 files changed, 136 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH v4 1/2] perf util: Check for fused instruction

[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:
-----------
v4: Move the CPU model checking to symbol__disassemble and
    save the CPU family/model in arch structure.

    It avoids checking every time when jump arrow printed.

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 | 46 +++++++++++++++++++++++++++++
 tools/perf/builtin-top.c                    |  2 +-
 tools/perf/ui/browsers/annotate.c           |  4 ++-
 tools/perf/ui/gtk/annotate.c                |  2 +-
 tools/perf/util/annotate.c                  | 22 ++++++++++++--
 tools/perf/util/annotate.h                  |  3 +-
 6 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index c1625f2..d84b720 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -76,3 +76,49 @@ static struct ins x86__instructions[] = {
 	{ .name = "xbeginq",	.ops = &jump_ops, },
 	{ .name = "retq",	.ops = &ret_ops,  },
 };
+
+static bool x86__ins_is_fused(struct arch *arch, const char *ins1,
+			      const char *ins2)
+{
+	if (arch->family != 6 || arch->model < 0x1e || strstr(ins2, "jmp"))
+		return false;
+
+	if (arch->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;
+}
+
+static int x86__cpuid_parse(struct arch *arch, char *cpuid)
+{
+	unsigned int family, model, stepping;
+	int ret;
+
+	/*
+	 * cpuid = "GenuineIntel,family,model,stepping"
+	 */
+	ret = sscanf(cpuid, "%*[^,],%u,%u,%u", &family, &model, &stepping);
+	if (ret == 3) {
+		arch->family = family;
+		arch->model = model;
+		return 0;
+	}
+
+	return -1;
+}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6052376..022486d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -134,7 +134,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)
 		return err;
 	}
 
-	err = symbol__disassemble(sym, map, NULL, 0, NULL);
+	err = symbol__disassemble(sym, map, NULL, 0, NULL, NULL);
 	if (err == 0) {
 out_assign:
 		top->sym_filter_entry = he;
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 27f41f2..ae05ebb 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>
@@ -1074,7 +1075,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 	}
 
 	err = symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
-				  sizeof_bdl, &browser.arch);
+				  sizeof_bdl, &browser.arch,
+				  evsel->evlist->env->cpuid);
 	if (err) {
 		char msg[BUFSIZ];
 		symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index d903fd4..87e3760 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -169,7 +169,7 @@ static int symbol__gtk_annotate(struct symbol *sym, struct map *map,
 		return -1;
 
 	err = symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
-				  0, NULL);
+				  0, NULL, NULL);
 	if (err) {
 		char msg[BUFSIZ];
 		symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index be1caab..8748ebb 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -47,7 +47,12 @@ struct arch {
 	bool		sorted_instructions;
 	bool		initialized;
 	void		*priv;
+	unsigned int	model;
+	unsigned int	family;
 	int		(*init)(struct arch *arch);
+	bool		(*ins_is_fused)(struct arch *arch, const char *ins1,
+					const char *ins2);
+	int		(*cpuid_parse)(struct arch *arch, char *cpuid);
 	struct		{
 		char comment_char;
 		char skip_functions_char;
@@ -129,6 +134,8 @@ static struct arch architectures[] = {
 		.name = "x86",
 		.instructions = x86__instructions,
 		.nr_instructions = ARRAY_SIZE(x86__instructions),
+		.ins_is_fused = x86__ins_is_fused,
+		.cpuid_parse = x86__cpuid_parse,
 		.objdump =  {
 			.comment_char = '#',
 		},
@@ -171,6 +178,14 @@ 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, const char *ins1, const char *ins2)
+{
+	if (!arch || !arch->ins_is_fused)
+		return false;
+
+	return arch->ins_is_fused(arch, ins1, ins2);
+}
+
 static int call__parse(struct arch *arch, struct ins_operands *ops, struct map *map)
 {
 	char *endptr, *tok, *name;
@@ -1381,7 +1396,7 @@ static const char *annotate__norm_arch(const char *arch_name)
 
 int symbol__disassemble(struct symbol *sym, struct map *map,
 			const char *arch_name, size_t privsize,
-			struct arch **parch)
+			struct arch **parch, char *cpuid)
 {
 	struct dso *dso = map->dso;
 	char command[PATH_MAX * 2];
@@ -1418,6 +1433,9 @@ int symbol__disassemble(struct symbol *sym, struct map *map,
 		}
 	}
 
+	if (arch->cpuid_parse && cpuid)
+		arch->cpuid_parse(arch, cpuid);
+
 	pr_debug("%s: filename=%s, sym=%s, start=%#" PRIx64 ", end=%#" PRIx64 "\n", __func__,
 		 symfs_filename, sym->name, map->unmap_ip(map, sym->start),
 		 map->unmap_ip(map, sym->end));
@@ -1907,7 +1925,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
 	u64 len;
 
 	if (symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
-				0, NULL) < 0)
+				0, NULL, NULL) < 0)
 		return -1;
 
 	len = symbol__size(sym);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 2105503..72d7272 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -53,6 +53,7 @@ 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, const char *ins1, const char *ins2);
 
 struct annotation;
 
@@ -160,7 +161,7 @@ void symbol__annotate_zero_histograms(struct symbol *sym);
 
 int symbol__disassemble(struct symbol *sym, struct map *map,
 			const char *arch_name, size_t privsize,
-			struct arch **parch);
+			struct arch **parch, char *cpuid);
 
 enum symbol_disassemble_errno {
 	SYMBOL_ANNOTATE_ERRNO__SUCCESS		= 0,
-- 
2.7.4

[PATCH v4 2/2] perf report: Implement visual marker for macro fusion in annotate

[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:
-----------
v4: Since the first patch in patch set is changed, this is mainly
    changed for compilation.

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 | 26 ++++++++++++++++++++++++++
 tools/perf/util/annotate.c        |  5 +++++
 tools/perf/util/annotate.h        |  1 +
 5 files changed, 63 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 ae05ebb..b376637 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -273,6 +273,25 @@ 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);
+	const char *name;
+
+	if (!pos)
+		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, 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);
@@ -308,6 +327,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)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 8748ebb..ef434b5 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -517,6 +517,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 72d7272..bac698d 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, const char *ins1, const char *ins2);
 
-- 
2.7.4

Re: [PATCH v4 2/2] perf report: Implement visual marker for macro fusion in annotate

[Arnaldo Carvalho de Melo]

Em Fri, Jul 07, 2017 at 01:06:35PM +0800, Jin Yao escreveu:
> 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.

I applied this one, no unnecessary parsing of cpuid done at each
jump->target arrow rendering, much better, thanks!

One thing for a follow up patch:

We have this when the cursor is at a jump instruction:

       │      ┌──test   %ecx,%ecx
->     │      ├──je     714cf
       │      │  mov    LINES+0xb40,%edx
       │      │  test   %edx,%edx
       │      │↓ je     71580
       │714cf:└─→mov    LINES+0x10c8,%eax

But if we go up a line, to that "test" instruction, we get:

->     │         test   %ecx,%ecx
       │       ↓ je     714cf
       │         mov    LINES+0xb40,%edx
       │         test   %edx,%edx
       │       ↓ je     71580
       │714cf:   mov    LINES+0x10c8,%eax

I suggest that this be changed to:

->     │       ┌─test   %ecx,%ecx
       │       ↓ je     714cf
       │         mov    LINES+0xb40,%edx
       │         test   %edx,%edx
       │       ↓ je     71580
       │714cf:   mov    LINES+0x10c8,%eax

I.e. even before going to the jump instruction line with the cursor, we
would see the fused instructions.

To do that perhaps we should improve annotate_browser__draw_current_jump
to improve that part that looks for is_valid_jump() to consider
instructions that could be fused with jumps for the machine where the
perf data came from, etc.

But the current situation is better already, thanks for your work,
applied!

- Arnaldo

Re: [PATCH v4 2/2] perf report: Implement visual marker for macro fusion in annotate

[Jin, Yao]

> I applied this one, no unnecessary parsing of cpuid done at each
> jump->target arrow rendering, much better, thanks!
>
> One thing for a follow up patch:
>
> We have this when the cursor is at a jump instruction:
>
>         │      ┌──test   %ecx,%ecx
> ->     │      ├──je     714cf
>         │      │  mov    LINES+0xb40,%edx
>         │      │  test   %edx,%edx
>         │      │↓ je     71580
>         │714cf:└─→mov    LINES+0x10c8,%eax
>
> But if we go up a line, to that "test" instruction, we get:
>
> ->     │         test   %ecx,%ecx
>         │       ↓ je     714cf
>         │         mov    LINES+0xb40,%edx
>         │         test   %edx,%edx
>         │       ↓ je     71580
>         │714cf:   mov    LINES+0x10c8,%eax
>
> I suggest that this be changed to:
>
> ->     │       ┌─test   %ecx,%ecx
>         │       ↓ je     714cf
>         │         mov    LINES+0xb40,%edx
>         │         test   %edx,%edx
>         │       ↓ je     71580
>         │714cf:   mov    LINES+0x10c8,%eax
>
> I.e. even before going to the jump instruction line with the cursor, we
> would see the fused instructions.
>
> To do that perhaps we should improve annotate_browser__draw_current_jump
> to improve that part that looks for is_valid_jump() to consider
> instructions that could be fused with jumps for the machine where the
> perf data came from, etc.
>
> But the current situation is better already, thanks for your work,
> applied!
>
> - Arnaldo

I will investigate how to do the follow-up patch.

Thanks
Jin Yao

[tip:perf/core] perf annotate: Check for fused instructions

[tip-bot for Jin Yao]

Commit-ID:  69fb09f6ccdb2f070557fd1f4c56c4d646694c8e
Gitweb:     http://git.kernel.org/tip/69fb09f6ccdb2f070557fd1f4c56c4d646694c8e
Author:     Jin Yao <yao.jin@linux.intel.com>
AuthorDate: Fri, 7 Jul 2017 13:06:34 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 18 Jul 2017 23:11:25 -0300

perf annotate: Check for fused instructions

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.

Changelog:

v4: Move the CPU model checking to symbol__disassemble and save the CPU
    family/model in arch structure.

    It avoids checking every time when jump arrow printed.

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 original 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.

Committer fix:

Do not access evsel->evlist->env->cpuid, ->env can be null, introduce
perf_evsel__env_cpuid(), just like perf_evsel__env_arch(), also used in
this function call.

The original patch was segfaulting 'perf top' + annotation.

But this essentially disables this fused instructions augmentation in
'perf top', the right thing is to get the cpuid from the running kernel,
left for a later patch tho.

Signed-off-by: Yao Jin <yao.jin@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.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/1499403995-19857-2-git-send-email-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/x86/annotate/instructions.c | 46 +++++++++++++++++++++++++++++
 tools/perf/builtin-top.c                    |  2 +-
 tools/perf/ui/browsers/annotate.c           |  4 ++-
 tools/perf/ui/gtk/annotate.c                |  2 +-
 tools/perf/util/annotate.c                  | 22 ++++++++++++--
 tools/perf/util/annotate.h                  |  3 +-
 tools/perf/util/evsel.c                     |  7 +++++
 tools/perf/util/evsel.h                     |  1 +
 8 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index c1625f2..d84b720 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -76,3 +76,49 @@ static struct ins x86__instructions[] = {
 	{ .name = "xbeginq",	.ops = &jump_ops, },
 	{ .name = "retq",	.ops = &ret_ops,  },
 };
+
+static bool x86__ins_is_fused(struct arch *arch, const char *ins1,
+			      const char *ins2)
+{
+	if (arch->family != 6 || arch->model < 0x1e || strstr(ins2, "jmp"))
+		return false;
+
+	if (arch->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;
+}
+
+static int x86__cpuid_parse(struct arch *arch, char *cpuid)
+{
+	unsigned int family, model, stepping;
+	int ret;
+
+	/*
+	 * cpuid = "GenuineIntel,family,model,stepping"
+	 */
+	ret = sscanf(cpuid, "%*[^,],%u,%u,%u", &family, &model, &stepping);
+	if (ret == 3) {
+		arch->family = family;
+		arch->model = model;
+		return 0;
+	}
+
+	return -1;
+}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6052376..022486d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -134,7 +134,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)
 		return err;
 	}
 
-	err = symbol__disassemble(sym, map, NULL, 0, NULL);
+	err = symbol__disassemble(sym, map, NULL, 0, NULL, NULL);
 	if (err == 0) {
 out_assign:
 		top->sym_filter_entry = he;
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 27f41f2..c433613 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>
@@ -1074,7 +1075,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 	}
 
 	err = symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
-				  sizeof_bdl, &browser.arch);
+				  sizeof_bdl, &browser.arch,
+				  perf_evsel__env_cpuid(evsel));
 	if (err) {
 		char msg[BUFSIZ];
 		symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index d903fd4..87e3760 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -169,7 +169,7 @@ static int symbol__gtk_annotate(struct symbol *sym, struct map *map,
 		return -1;
 
 	err = symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
-				  0, NULL);
+				  0, NULL, NULL);
 	if (err) {
 		char msg[BUFSIZ];
 		symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index be1caab..8748ebb 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -47,7 +47,12 @@ struct arch {
 	bool		sorted_instructions;
 	bool		initialized;
 	void		*priv;
+	unsigned int	model;
+	unsigned int	family;
 	int		(*init)(struct arch *arch);
+	bool		(*ins_is_fused)(struct arch *arch, const char *ins1,
+					const char *ins2);
+	int		(*cpuid_parse)(struct arch *arch, char *cpuid);
 	struct		{
 		char comment_char;
 		char skip_functions_char;
@@ -129,6 +134,8 @@ static struct arch architectures[] = {
 		.name = "x86",
 		.instructions = x86__instructions,
 		.nr_instructions = ARRAY_SIZE(x86__instructions),
+		.ins_is_fused = x86__ins_is_fused,
+		.cpuid_parse = x86__cpuid_parse,
 		.objdump =  {
 			.comment_char = '#',
 		},
@@ -171,6 +178,14 @@ 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, const char *ins1, const char *ins2)
+{
+	if (!arch || !arch->ins_is_fused)
+		return false;
+
+	return arch->ins_is_fused(arch, ins1, ins2);
+}
+
 static int call__parse(struct arch *arch, struct ins_operands *ops, struct map *map)
 {
 	char *endptr, *tok, *name;
@@ -1381,7 +1396,7 @@ static const char *annotate__norm_arch(const char *arch_name)
 
 int symbol__disassemble(struct symbol *sym, struct map *map,
 			const char *arch_name, size_t privsize,
-			struct arch **parch)
+			struct arch **parch, char *cpuid)
 {
 	struct dso *dso = map->dso;
 	char command[PATH_MAX * 2];
@@ -1418,6 +1433,9 @@ int symbol__disassemble(struct symbol *sym, struct map *map,
 		}
 	}
 
+	if (arch->cpuid_parse && cpuid)
+		arch->cpuid_parse(arch, cpuid);
+
 	pr_debug("%s: filename=%s, sym=%s, start=%#" PRIx64 ", end=%#" PRIx64 "\n", __func__,
 		 symfs_filename, sym->name, map->unmap_ip(map, sym->start),
 		 map->unmap_ip(map, sym->end));
@@ -1907,7 +1925,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
 	u64 len;
 
 	if (symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
-				0, NULL) < 0)
+				0, NULL, NULL) < 0)
 		return -1;
 
 	len = symbol__size(sym);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 2105503..72d7272 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -53,6 +53,7 @@ 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, const char *ins1, const char *ins2);
 
 struct annotation;
 
@@ -160,7 +161,7 @@ void symbol__annotate_zero_histograms(struct symbol *sym);
 
 int symbol__disassemble(struct symbol *sym, struct map *map,
 			const char *arch_name, size_t privsize,
-			struct arch **parch);
+			struct arch **parch, char *cpuid);
 
 enum symbol_disassemble_errno {
 	SYMBOL_ANNOTATE_ERRNO__SUCCESS		= 0,
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 413f74d..0e4cd60 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2610,3 +2610,10 @@ char *perf_evsel__env_arch(struct perf_evsel *evsel)
 		return evsel->evlist->env->arch;
 	return NULL;
 }
+
+char *perf_evsel__env_cpuid(struct perf_evsel *evsel)
+{
+	if (evsel && evsel->evlist && evsel->evlist->env)
+		return evsel->evlist->env->cpuid;
+	return NULL;
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index d101695..219ad0c 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -436,5 +436,6 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 			     attr__fprintf_f attr__fprintf, void *priv);
 
 char *perf_evsel__env_arch(struct perf_evsel *evsel);
+char *perf_evsel__env_cpuid(struct perf_evsel *evsel);
 
 #endif /* __PERF_EVSEL_H */

[tip:perf/core] perf annotate: Implement visual marker for macro fusion

[tip-bot for Jin Yao]

Commit-ID:  7e63a13a266da652f82731b845b5c35dd866ec7e
Gitweb:     http://git.kernel.org/tip/7e63a13a266da652f82731b845b5c35dd866ec7e
Author:     Jin Yao <yao.jin@linux.intel.com>
AuthorDate: Fri, 7 Jul 2017 13:06:35 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 18 Jul 2017 23:13:49 -0300

perf annotate: Implement visual marker for macro fusion

For marking fused instructions clearly this patch adds a line before the
first instruction of pair and joins it with the arrow of the jump to its
target.

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 a fused instruction pair and they should be
considered together.

Changelog:

v3: Use Arnaldo's fix to improve the arrow origin rendering.  To get the
    evsel->evlist->env->cpuid, save the evsel in annotate_browser.

v2: new function "ins__is_fused" to check if the instructions are fused.

Signed-off-by: Yao Jin <yao.jin@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.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/1499403995-19857-3-git-send-email-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browser.c           | 29 +++++++++++++++++++++++++++++
 tools/perf/ui/browser.h           |  2 ++
 tools/perf/ui/browsers/annotate.c | 26 ++++++++++++++++++++++++++
 tools/perf/util/annotate.c        |  5 +++++
 tools/perf/util/annotate.h        |  1 +
 5 files changed, 63 insertions(+)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 83874b0..f73f3f1 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 c433613..8d3f6f5 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -273,6 +273,25 @@ 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);
+	const char *name;
+
+	if (!pos)
+		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, 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);
@@ -308,6 +327,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)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 8748ebb..ef434b5 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -517,6 +517,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 72d7272..bac698d 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, const char *ins1, const char *ins2);