linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] perf annotate: Support for AArch64
@ 2016-05-19 16:59 Chris Ryder
  2016-05-19 16:59 ` [PATCH 1/7] perf annotate: Fix identification of ARM blt and bls instructions Chris Ryder
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

Hi,

The linux perf tool has some basic support for annotating
AArch32 branch instructions when displaying assembly. This
patchset refactors the annotation support to have a cleaner
separation between architectures, and then adds support for
annotating AArch64 instructions.

Comments very welcome, thanks.
Chris.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>

Chris Ryder (7):
  perf annotate: Fix identification of ARM blt and bls instructions
  perf annotate: Sort list of recognised instructions
  pref annotate: Separate architecture specific annotation support
  perf annotate: Separate out architecture specific parsing
  perf annotate: Architecture neutral handling of return instruction
  perf annotate: Make action message be architecture specific
  perf annotate: AArch64 support

 tools/perf/arch/arm/include/annotate_ins.h   |  27 +++++
 tools/perf/arch/arm/util/Build               |   2 +
 tools/perf/arch/arm/util/annotate_ins.c      |  22 ++++
 tools/perf/arch/arm64/include/annotate_ins.h |  40 +++++++
 tools/perf/arch/arm64/util/Build             |   2 +
 tools/perf/arch/arm64/util/annotate_ins.c    |  21 ++++
 tools/perf/arch/x86/include/annotate_ins.h   |  84 +++++++++++++++
 tools/perf/arch/x86/util/Build               |   1 +
 tools/perf/arch/x86/util/annotate_ins.c      |  17 +++
 tools/perf/config/Makefile                   |  12 +++
 tools/perf/ui/browsers/annotate.c            |  15 +--
 tools/perf/util/Build                        |   1 +
 tools/perf/util/annotate.c                   | 150 +++++++--------------------
 tools/perf/util/annotate_ins.c               |  21 ++++
 tools/perf/util/annotate_ins.h               |  17 +++
 15 files changed, 313 insertions(+), 119 deletions(-)
 create mode 100644 tools/perf/arch/arm/include/annotate_ins.h
 create mode 100644 tools/perf/arch/arm/util/annotate_ins.c
 create mode 100644 tools/perf/arch/arm64/include/annotate_ins.h
 create mode 100644 tools/perf/arch/arm64/util/annotate_ins.c
 create mode 100644 tools/perf/arch/x86/include/annotate_ins.h
 create mode 100644 tools/perf/arch/x86/util/annotate_ins.c
 create mode 100644 tools/perf/util/annotate_ins.c
 create mode 100644 tools/perf/util/annotate_ins.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/7] perf annotate: Fix identification of ARM blt and bls instructions
  2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
@ 2016-05-19 16:59 ` Chris Ryder
  2016-05-20 17:45   ` [tip:perf/urgent] " tip-bot for Chris Ryder
  2016-05-19 16:59 ` [PATCH 2/7] perf annotate: Sort list of recognised instructions Chris Ryder
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

The ARM blt and bls instructions are not correctly identified when
parsing assembly because the list of recognised instructions must
be sorted by name. Swap the ordering of blt and bls.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 tools/perf/util/annotate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 4db73d5..d197571 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -372,8 +372,8 @@ static struct ins instructions[] = {
 	{ .name = "bgt",   .ops  = &jump_ops, },
 	{ .name = "bhi",   .ops  = &jump_ops, },
 	{ .name = "bl",    .ops  = &call_ops, },
-	{ .name = "blt",   .ops  = &jump_ops, },
 	{ .name = "bls",   .ops  = &jump_ops, },
+	{ .name = "blt",   .ops  = &jump_ops, },
 	{ .name = "blx",   .ops  = &call_ops, },
 	{ .name = "bne",   .ops  = &jump_ops, },
 #endif
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/7] perf annotate: Sort list of recognised instructions
  2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
  2016-05-19 16:59 ` [PATCH 1/7] perf annotate: Fix identification of ARM blt and bls instructions Chris Ryder
@ 2016-05-19 16:59 ` Chris Ryder
  2016-05-20 17:45   ` [tip:perf/urgent] " tip-bot for Chris Ryder
  2016-05-19 16:59 ` [PATCH 3/7] pref annotate: Separate architecture specific annotation support Chris Ryder
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

Currently the list of instructions recognised by perf annotate
has to be explicitly written in sorted order. This makes it easy
to make mistakes when adding new instructions. Sort the list of
instructions on first access.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 tools/perf/util/annotate.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index d197571..619bc27 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -354,9 +354,6 @@ static struct ins_ops nop_ops = {
 	.scnprintf = nop__scnprintf,
 };
 
-/*
- * Must be sorted by name!
- */
 static struct ins instructions[] = {
 	{ .name = "add",   .ops  = &mov_ops, },
 	{ .name = "addl",  .ops  = &mov_ops, },
@@ -449,18 +446,39 @@ static struct ins instructions[] = {
 	{ .name = "xbeginq", .ops  = &jump_ops, },
 };
 
-static int ins__cmp(const void *name, const void *insp)
+static int ins__key_cmp(const void *name, const void *insp)
 {
 	const struct ins *ins = insp;
 
 	return strcmp(name, ins->name);
 }
 
+static int ins__cmp(const void *a, const void *b)
+{
+	const struct ins *ia = a;
+	const struct ins *ib = b;
+
+	return strcmp(ia->name, ib->name);
+}
+
+static void ins__sort(void)
+{
+	const int nmemb = ARRAY_SIZE(instructions);
+
+	qsort(instructions, nmemb, sizeof(struct ins), ins__cmp);
+}
+
 static struct ins *ins__find(const char *name)
 {
 	const int nmemb = ARRAY_SIZE(instructions);
+	static bool sorted;
+
+	if (!sorted) {
+		ins__sort();
+		sorted = true;
+	}
 
-	return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__cmp);
+	return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__key_cmp);
 }
 
 int symbol__annotate_init(struct map *map __maybe_unused, struct symbol *sym)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/7] pref annotate: Separate architecture specific annotation support
  2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
  2016-05-19 16:59 ` [PATCH 1/7] perf annotate: Fix identification of ARM blt and bls instructions Chris Ryder
  2016-05-19 16:59 ` [PATCH 2/7] perf annotate: Sort list of recognised instructions Chris Ryder
@ 2016-05-19 16:59 ` Chris Ryder
  2016-05-19 19:45   ` Arnaldo Carvalho de Melo
  2016-05-19 16:59 ` [PATCH 4/7] perf annotate: Separate out architecture specific parsing Chris Ryder
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

Currently the list of instructions recognised by perf annotate
contains an intermingled mix of x86 and ARM instructions, and the x86
instructions are unconditionally included on all platforms. This means
that perf attempts to parse x86 instructions on non-x86 platforms.
Refactor the instruction list into per-architecture header files so that
only the instructions applicable to the target architecture are parsed.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 tools/perf/arch/arm/include/annotate_ins.h |  25 +++++++
 tools/perf/arch/x86/include/annotate_ins.h |  82 ++++++++++++++++++++++
 tools/perf/config/Makefile                 |  11 +++
 tools/perf/util/annotate.c                 | 105 +++--------------------------
 tools/perf/util/annotate_ins.h             |  10 +++
 5 files changed, 136 insertions(+), 97 deletions(-)
 create mode 100644 tools/perf/arch/arm/include/annotate_ins.h
 create mode 100644 tools/perf/arch/x86/include/annotate_ins.h
 create mode 100644 tools/perf/util/annotate_ins.h

diff --git a/tools/perf/arch/arm/include/annotate_ins.h b/tools/perf/arch/arm/include/annotate_ins.h
new file mode 100644
index 0000000..e8ea98d
--- /dev/null
+++ b/tools/perf/arch/arm/include/annotate_ins.h
@@ -0,0 +1,25 @@
+#ifndef ARCH_ANNOTATE_INS_H
+#define ARCH_ANNOTATE_INS_H
+
+#define ARCH_INSTRUCTIONS { \
+	{ .name = "add",   .ops  = &mov_ops, }, \
+	{ .name = "and",   .ops  = &mov_ops, }, \
+	{ .name = "b",     .ops  = &jump_ops, }, /* might also be a call */ \
+	{ .name = "bcc",   .ops  = &jump_ops, }, \
+	{ .name = "bcs",   .ops  = &jump_ops, }, \
+	{ .name = "beq",   .ops  = &jump_ops, }, \
+	{ .name = "bge",   .ops  = &jump_ops, }, \
+	{ .name = "bgt",   .ops  = &jump_ops, }, \
+	{ .name = "bhi",   .ops  = &jump_ops, }, \
+	{ .name = "bl",    .ops  = &call_ops, }, \
+	{ .name = "bls",   .ops  = &jump_ops, }, \
+	{ .name = "blt",   .ops  = &jump_ops, }, \
+	{ .name = "blx",   .ops  = &call_ops, }, \
+	{ .name = "bne",   .ops  = &jump_ops, }, \
+	{ .name = "cmp",   .ops  = &mov_ops, }, \
+	{ .name = "mov",   .ops  = &mov_ops, }, \
+	{ .name = "nop",   .ops  = &nop_ops, }, \
+	{ .name = "orr",   .ops  = &mov_ops, }, \
+	}
+
+#endif /* ARCH_ANNOTATE_INS_H */
diff --git a/tools/perf/arch/x86/include/annotate_ins.h b/tools/perf/arch/x86/include/annotate_ins.h
new file mode 100644
index 0000000..179b50e
--- /dev/null
+++ b/tools/perf/arch/x86/include/annotate_ins.h
@@ -0,0 +1,82 @@
+#ifndef ARCH_ANNOTATE_INS_H
+#define ARCH_ANNOTATE_INS_H
+
+#define ARCH_INSTRUCTIONS { \
+	{ .name = "add",     .ops  = &mov_ops, }, \
+	{ .name = "addl",    .ops  = &mov_ops, }, \
+	{ .name = "addq",    .ops  = &mov_ops, }, \
+	{ .name = "addw",    .ops  = &mov_ops, }, \
+	{ .name = "and",     .ops  = &mov_ops, }, \
+	{ .name = "bts",     .ops  = &mov_ops, }, \
+	{ .name = "call",    .ops  = &call_ops, }, \
+	{ .name = "callq",   .ops  = &call_ops, }, \
+	{ .name = "cmp",     .ops  = &mov_ops, }, \
+	{ .name = "cmpb",    .ops  = &mov_ops, }, \
+	{ .name = "cmpl",    .ops  = &mov_ops, }, \
+	{ .name = "cmpq",    .ops  = &mov_ops, }, \
+	{ .name = "cmpw",    .ops  = &mov_ops, }, \
+	{ .name = "cmpxch",  .ops  = &mov_ops, }, \
+	{ .name = "dec",     .ops  = &dec_ops, }, \
+	{ .name = "decl",    .ops  = &dec_ops, }, \
+	{ .name = "imul",    .ops  = &mov_ops, }, \
+	{ .name = "inc",     .ops  = &dec_ops, }, \
+	{ .name = "incl",    .ops  = &dec_ops, }, \
+	{ .name = "ja",	     .ops  = &jump_ops, }, \
+	{ .name = "jae",     .ops  = &jump_ops, }, \
+	{ .name = "jb",	     .ops  = &jump_ops, }, \
+	{ .name = "jbe",     .ops  = &jump_ops, }, \
+	{ .name = "jc",	     .ops  = &jump_ops, }, \
+	{ .name = "jcxz",    .ops  = &jump_ops, }, \
+	{ .name = "je",	     .ops  = &jump_ops, }, \
+	{ .name = "jecxz",   .ops  = &jump_ops, }, \
+	{ .name = "jg",	     .ops  = &jump_ops, }, \
+	{ .name = "jge",     .ops  = &jump_ops, }, \
+	{ .name = "jl",      .ops  = &jump_ops, }, \
+	{ .name = "jle",     .ops  = &jump_ops, }, \
+	{ .name = "jmp",     .ops  = &jump_ops, }, \
+	{ .name = "jmpq",    .ops  = &jump_ops, }, \
+	{ .name = "jna",     .ops  = &jump_ops, }, \
+	{ .name = "jnae",    .ops  = &jump_ops, }, \
+	{ .name = "jnb",     .ops  = &jump_ops, }, \
+	{ .name = "jnbe",    .ops  = &jump_ops, }, \
+	{ .name = "jnc",     .ops  = &jump_ops, }, \
+	{ .name = "jne",     .ops  = &jump_ops, }, \
+	{ .name = "jng",     .ops  = &jump_ops, }, \
+	{ .name = "jnge",    .ops  = &jump_ops, }, \
+	{ .name = "jnl",     .ops  = &jump_ops, }, \
+	{ .name = "jnle",    .ops  = &jump_ops, }, \
+	{ .name = "jno",     .ops  = &jump_ops, }, \
+	{ .name = "jnp",     .ops  = &jump_ops, }, \
+	{ .name = "jns",     .ops  = &jump_ops, }, \
+	{ .name = "jnz",     .ops  = &jump_ops, }, \
+	{ .name = "jo",	     .ops  = &jump_ops, }, \
+	{ .name = "jp",	     .ops  = &jump_ops, }, \
+	{ .name = "jpe",     .ops  = &jump_ops, }, \
+	{ .name = "jpo",     .ops  = &jump_ops, }, \
+	{ .name = "jrcxz",   .ops  = &jump_ops, }, \
+	{ .name = "js",	     .ops  = &jump_ops, }, \
+	{ .name = "jz",	     .ops  = &jump_ops, }, \
+	{ .name = "lea",     .ops  = &mov_ops, }, \
+	{ .name = "lock",    .ops  = &lock_ops, }, \
+	{ .name = "mov",     .ops  = &mov_ops, }, \
+	{ .name = "movb",    .ops  = &mov_ops, }, \
+	{ .name = "movdqa",  .ops  = &mov_ops, }, \
+	{ .name = "movl",    .ops  = &mov_ops, }, \
+	{ .name = "movq",    .ops  = &mov_ops, }, \
+	{ .name = "movslq",  .ops  = &mov_ops, }, \
+	{ .name = "movzbl",  .ops  = &mov_ops, }, \
+	{ .name = "movzwl",  .ops  = &mov_ops, }, \
+	{ .name = "nop",     .ops  = &nop_ops, }, \
+	{ .name = "nopl",    .ops  = &nop_ops, }, \
+	{ .name = "nopw",    .ops  = &nop_ops, }, \
+	{ .name = "or",      .ops  = &mov_ops, }, \
+	{ .name = "orl",     .ops  = &mov_ops, }, \
+	{ .name = "test",    .ops  = &mov_ops, }, \
+	{ .name = "testb",   .ops  = &mov_ops, }, \
+	{ .name = "testl",   .ops  = &mov_ops, }, \
+	{ .name = "xadd",    .ops  = &mov_ops, }, \
+	{ .name = "xbeginl", .ops  = &jump_ops, }, \
+	{ .name = "xbeginq", .ops  = &jump_ops, }, \
+	}
+
+#endif /* ARCH_ANNOTATE_INS_H */
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 1e46277..d3eba89 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -22,6 +22,7 @@ include $(srctree)/tools/scripts/Makefile.arch
 $(call detected_var,ARCH)
 
 NO_PERF_REGS := 1
+NO_ANNOTATE_INS := 1
 
 # Additional ARCH settings for x86
 ifeq ($(ARCH),x86)
@@ -35,10 +36,12 @@ ifeq ($(ARCH),x86)
     LIBUNWIND_LIBS = -lunwind-x86 -llzma -lunwind
   endif
   NO_PERF_REGS := 0
+  NO_ANNOTATE_INS := 0
 endif
 
 ifeq ($(ARCH),arm)
   NO_PERF_REGS := 0
+  NO_ANNOTATE_INS := 0
   LIBUNWIND_LIBS = -lunwind -lunwind-arm
 endif
 
@@ -51,6 +54,10 @@ ifeq ($(NO_PERF_REGS),0)
   $(call detected,CONFIG_PERF_REGS)
 endif
 
+ifeq ($(NO_ANNOTATE_INS),0)
+  $(call detected,CONFIG_ANNOTATE_INS)
+endif
+
 # So far there's only x86 and arm libdw unwind support merged in perf.
 # Disable it on all other architectures in case libdw unwind
 # support is detected in system. Add supported architectures
@@ -83,6 +90,10 @@ ifeq ($(NO_PERF_REGS),0)
   CFLAGS += -DHAVE_PERF_REGS_SUPPORT
 endif
 
+ifeq ($(NO_ANNOTATE_INS),0)
+  CFLAGS += -DHAVE_ANNOTATE_INS_SUPPORT
+endif
+
 # for linking with debug library, run like:
 # make DEBUG=1 LIBDW_DIR=/opt/libdw/
 ifdef LIBDW_DIR
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 619bc27..71c1dd5 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -20,6 +20,7 @@
 #include <regex.h>
 #include <pthread.h>
 #include <linux/bitops.h>
+#include "annotate_ins.h"
 
 const char 	*disassembler_style;
 const char	*objdump_path;
@@ -107,7 +108,7 @@ static int call__scnprintf(struct ins *ins, char *bf, size_t size,
 	return scnprintf(bf, size, "%-6.6s *%" PRIx64, ins->name, ops->target.addr);
 }
 
-static struct ins_ops call_ops = {
+static struct ins_ops call_ops __maybe_unused = {
 	.parse	   = call__parse,
 	.scnprintf = call__scnprintf,
 };
@@ -137,7 +138,7 @@ static int jump__scnprintf(struct ins *ins, char *bf, size_t size,
 	return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, ops->target.offset);
 }
 
-static struct ins_ops jump_ops = {
+static struct ins_ops jump_ops __maybe_unused = {
 	.parse	   = jump__parse,
 	.scnprintf = jump__scnprintf,
 };
@@ -230,7 +231,7 @@ static void lock__delete(struct ins_operands *ops)
 	zfree(&ops->target.name);
 }
 
-static struct ins_ops lock_ops = {
+static struct ins_ops lock_ops __maybe_unused = {
 	.free	   = lock__delete,
 	.parse	   = lock__parse,
 	.scnprintf = lock__scnprintf,
@@ -298,7 +299,7 @@ static int mov__scnprintf(struct ins *ins, char *bf, size_t size,
 			 ops->target.name ?: ops->target.raw);
 }
 
-static struct ins_ops mov_ops = {
+static struct ins_ops mov_ops __maybe_unused = {
 	.parse	   = mov__parse,
 	.scnprintf = mov__scnprintf,
 };
@@ -339,7 +340,7 @@ static int dec__scnprintf(struct ins *ins, char *bf, size_t size,
 			 ops->target.name ?: ops->target.raw);
 }
 
-static struct ins_ops dec_ops = {
+static struct ins_ops dec_ops __maybe_unused = {
 	.parse	   = dec__parse,
 	.scnprintf = dec__scnprintf,
 };
@@ -350,101 +351,11 @@ static int nop__scnprintf(struct ins *ins __maybe_unused, char *bf, size_t size,
 	return scnprintf(bf, size, "%-6.6s", "nop");
 }
 
-static struct ins_ops nop_ops = {
+static struct ins_ops nop_ops __maybe_unused = {
 	.scnprintf = nop__scnprintf,
 };
 
-static struct ins instructions[] = {
-	{ .name = "add",   .ops  = &mov_ops, },
-	{ .name = "addl",  .ops  = &mov_ops, },
-	{ .name = "addq",  .ops  = &mov_ops, },
-	{ .name = "addw",  .ops  = &mov_ops, },
-	{ .name = "and",   .ops  = &mov_ops, },
-#ifdef __arm__
-	{ .name = "b",     .ops  = &jump_ops, }, // might also be a call
-	{ .name = "bcc",   .ops  = &jump_ops, },
-	{ .name = "bcs",   .ops  = &jump_ops, },
-	{ .name = "beq",   .ops  = &jump_ops, },
-	{ .name = "bge",   .ops  = &jump_ops, },
-	{ .name = "bgt",   .ops  = &jump_ops, },
-	{ .name = "bhi",   .ops  = &jump_ops, },
-	{ .name = "bl",    .ops  = &call_ops, },
-	{ .name = "bls",   .ops  = &jump_ops, },
-	{ .name = "blt",   .ops  = &jump_ops, },
-	{ .name = "blx",   .ops  = &call_ops, },
-	{ .name = "bne",   .ops  = &jump_ops, },
-#endif
-	{ .name = "bts",   .ops  = &mov_ops, },
-	{ .name = "call",  .ops  = &call_ops, },
-	{ .name = "callq", .ops  = &call_ops, },
-	{ .name = "cmp",   .ops  = &mov_ops, },
-	{ .name = "cmpb",  .ops  = &mov_ops, },
-	{ .name = "cmpl",  .ops  = &mov_ops, },
-	{ .name = "cmpq",  .ops  = &mov_ops, },
-	{ .name = "cmpw",  .ops  = &mov_ops, },
-	{ .name = "cmpxch", .ops  = &mov_ops, },
-	{ .name = "dec",   .ops  = &dec_ops, },
-	{ .name = "decl",  .ops  = &dec_ops, },
-	{ .name = "imul",  .ops  = &mov_ops, },
-	{ .name = "inc",   .ops  = &dec_ops, },
-	{ .name = "incl",  .ops  = &dec_ops, },
-	{ .name = "ja",	   .ops  = &jump_ops, },
-	{ .name = "jae",   .ops  = &jump_ops, },
-	{ .name = "jb",	   .ops  = &jump_ops, },
-	{ .name = "jbe",   .ops  = &jump_ops, },
-	{ .name = "jc",	   .ops  = &jump_ops, },
-	{ .name = "jcxz",  .ops  = &jump_ops, },
-	{ .name = "je",	   .ops  = &jump_ops, },
-	{ .name = "jecxz", .ops  = &jump_ops, },
-	{ .name = "jg",	   .ops  = &jump_ops, },
-	{ .name = "jge",   .ops  = &jump_ops, },
-	{ .name = "jl",    .ops  = &jump_ops, },
-	{ .name = "jle",   .ops  = &jump_ops, },
-	{ .name = "jmp",   .ops  = &jump_ops, },
-	{ .name = "jmpq",  .ops  = &jump_ops, },
-	{ .name = "jna",   .ops  = &jump_ops, },
-	{ .name = "jnae",  .ops  = &jump_ops, },
-	{ .name = "jnb",   .ops  = &jump_ops, },
-	{ .name = "jnbe",  .ops  = &jump_ops, },
-	{ .name = "jnc",   .ops  = &jump_ops, },
-	{ .name = "jne",   .ops  = &jump_ops, },
-	{ .name = "jng",   .ops  = &jump_ops, },
-	{ .name = "jnge",  .ops  = &jump_ops, },
-	{ .name = "jnl",   .ops  = &jump_ops, },
-	{ .name = "jnle",  .ops  = &jump_ops, },
-	{ .name = "jno",   .ops  = &jump_ops, },
-	{ .name = "jnp",   .ops  = &jump_ops, },
-	{ .name = "jns",   .ops  = &jump_ops, },
-	{ .name = "jnz",   .ops  = &jump_ops, },
-	{ .name = "jo",	   .ops  = &jump_ops, },
-	{ .name = "jp",	   .ops  = &jump_ops, },
-	{ .name = "jpe",   .ops  = &jump_ops, },
-	{ .name = "jpo",   .ops  = &jump_ops, },
-	{ .name = "jrcxz", .ops  = &jump_ops, },
-	{ .name = "js",	   .ops  = &jump_ops, },
-	{ .name = "jz",	   .ops  = &jump_ops, },
-	{ .name = "lea",   .ops  = &mov_ops, },
-	{ .name = "lock",  .ops  = &lock_ops, },
-	{ .name = "mov",   .ops  = &mov_ops, },
-	{ .name = "movb",  .ops  = &mov_ops, },
-	{ .name = "movdqa",.ops  = &mov_ops, },
-	{ .name = "movl",  .ops  = &mov_ops, },
-	{ .name = "movq",  .ops  = &mov_ops, },
-	{ .name = "movslq", .ops  = &mov_ops, },
-	{ .name = "movzbl", .ops  = &mov_ops, },
-	{ .name = "movzwl", .ops  = &mov_ops, },
-	{ .name = "nop",   .ops  = &nop_ops, },
-	{ .name = "nopl",  .ops  = &nop_ops, },
-	{ .name = "nopw",  .ops  = &nop_ops, },
-	{ .name = "or",    .ops  = &mov_ops, },
-	{ .name = "orl",   .ops  = &mov_ops, },
-	{ .name = "test",  .ops  = &mov_ops, },
-	{ .name = "testb", .ops  = &mov_ops, },
-	{ .name = "testl", .ops  = &mov_ops, },
-	{ .name = "xadd",  .ops  = &mov_ops, },
-	{ .name = "xbeginl", .ops  = &jump_ops, },
-	{ .name = "xbeginq", .ops  = &jump_ops, },
-};
+static struct ins instructions[] = ARCH_INSTRUCTIONS;
 
 static int ins__key_cmp(const void *name, const void *insp)
 {
diff --git a/tools/perf/util/annotate_ins.h b/tools/perf/util/annotate_ins.h
new file mode 100644
index 0000000..2ec9a05
--- /dev/null
+++ b/tools/perf/util/annotate_ins.h
@@ -0,0 +1,10 @@
+#ifndef __ANNOTATE_INS_H
+#define __ANNOTATE_INS_H
+
+#ifdef HAVE_ANNOTATE_INS_SUPPORT
+#include <annotate_ins.h>
+#else
+#define ARCH_INSTRUCTIONS { }
+#endif
+
+#endif /* __ANNOTATE_INS_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/7] perf annotate: Separate out architecture specific parsing
  2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
                   ` (2 preceding siblings ...)
  2016-05-19 16:59 ` [PATCH 3/7] pref annotate: Separate architecture specific annotation support Chris Ryder
@ 2016-05-19 16:59 ` Chris Ryder
  2016-05-19 16:59 ` [PATCH 5/7] perf annotate: Architecture neutral handling of return instruction Chris Ryder
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

Currently call__parse and mov__parse contain #ifdefs for ARM specific
parsing. Move the architecture specific parsing into the
per-architecture annotate_ins.h files.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 tools/perf/arch/arm/util/Build          |  2 ++
 tools/perf/arch/arm/util/annotate_ins.c | 15 +++++++++++++++
 tools/perf/arch/x86/util/Build          |  1 +
 tools/perf/arch/x86/util/annotate_ins.c | 12 ++++++++++++
 tools/perf/util/Build                   |  1 +
 tools/perf/util/annotate.c              | 17 +++++++----------
 tools/perf/util/annotate_ins.c          | 16 ++++++++++++++++
 tools/perf/util/annotate_ins.h          |  3 +++
 8 files changed, 57 insertions(+), 10 deletions(-)
 create mode 100644 tools/perf/arch/arm/util/annotate_ins.c
 create mode 100644 tools/perf/arch/x86/util/annotate_ins.c
 create mode 100644 tools/perf/util/annotate_ins.c

diff --git a/tools/perf/arch/arm/util/Build b/tools/perf/arch/arm/util/Build
index d22e3d0..61e4591 100644
--- a/tools/perf/arch/arm/util/Build
+++ b/tools/perf/arch/arm/util/Build
@@ -1,3 +1,5 @@
+libperf-y += annotate_ins.o
+
 libperf-$(CONFIG_DWARF) += dwarf-regs.o
 
 libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
diff --git a/tools/perf/arch/arm/util/annotate_ins.c b/tools/perf/arch/arm/util/annotate_ins.c
new file mode 100644
index 0000000..87fc691
--- /dev/null
+++ b/tools/perf/arch/arm/util/annotate_ins.c
@@ -0,0 +1,15 @@
+#include <string.h>
+#include <util/annotate_ins.h>
+
+char *arch_parse_mov_comment(const char *s)
+{
+	return strchr(s, ';');
+}
+
+char *arch_parse_call_target(char *t)
+{
+	if (strchr(t, '+'))
+		return NULL;
+
+	return t;
+}
diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
index 4659703..b7e6dfe 100644
--- a/tools/perf/arch/x86/util/Build
+++ b/tools/perf/arch/x86/util/Build
@@ -3,6 +3,7 @@ libperf-y += tsc.o
 libperf-y += pmu.o
 libperf-y += kvm-stat.o
 libperf-y += perf_regs.o
+libperf-y += annotate_ins.o
 
 libperf-$(CONFIG_DWARF) += dwarf-regs.o
 libperf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
diff --git a/tools/perf/arch/x86/util/annotate_ins.c b/tools/perf/arch/x86/util/annotate_ins.c
new file mode 100644
index 0000000..b007cd3
--- /dev/null
+++ b/tools/perf/arch/x86/util/annotate_ins.c
@@ -0,0 +1,12 @@
+#include <string.h>
+#include <util/annotate_ins.h>
+
+char *arch_parse_mov_comment(const char *s)
+{
+	return strchr(s, '#');
+}
+
+char *arch_parse_call_target(char *t)
+{
+	return t;
+}
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 8c6c8a0..8a1dd33 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -1,5 +1,6 @@
 libperf-y += alias.o
 libperf-y += annotate.o
+libperf-y += annotate_ins.o
 libperf-y += build-id.o
 libperf-y += config.o
 libperf-y += ctype.o
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 71c1dd5..ba04704 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -66,16 +66,16 @@ static int call__parse(struct ins_operands *ops)
 
 	name++;
 
-#ifdef __arm__
-	if (strchr(name, '+'))
-		return -1;
-#endif
-
 	tok = strchr(name, '>');
 	if (tok == NULL)
 		return -1;
 
 	*tok = '\0';
+
+	name = arch_parse_call_target(name);
+	if (name == NULL)
+		return -1;
+
 	ops->target.name = strdup(name);
 	*tok = '>';
 
@@ -252,11 +252,8 @@ static int mov__parse(struct ins_operands *ops)
 		return -1;
 
 	target = ++s;
-#ifdef __arm__
-	comment = strchr(s, ';');
-#else
-	comment = strchr(s, '#');
-#endif
+
+	comment = arch_parse_mov_comment(s);
 
 	if (comment != NULL)
 		s = comment - 1;
diff --git a/tools/perf/util/annotate_ins.c b/tools/perf/util/annotate_ins.c
new file mode 100644
index 0000000..3867545
--- /dev/null
+++ b/tools/perf/util/annotate_ins.c
@@ -0,0 +1,16 @@
+#ifndef HAVE_ANNOTATE_INS_SUPPORT
+
+#include <linux/compiler.h>
+#include <util/annotate_ins.h>
+
+char *arch_parse_mov_comment(const char *s __maybe_unused)
+{
+	return NULL;
+}
+
+char *arch_parse_call_target(char *t)
+{
+	return t;
+}
+
+#endif /* !HAVE_ANNOTATE_INS_SUPPORT */
diff --git a/tools/perf/util/annotate_ins.h b/tools/perf/util/annotate_ins.h
index 2ec9a05..a80f493 100644
--- a/tools/perf/util/annotate_ins.h
+++ b/tools/perf/util/annotate_ins.h
@@ -1,6 +1,9 @@
 #ifndef __ANNOTATE_INS_H
 #define __ANNOTATE_INS_H
 
+extern char *arch_parse_mov_comment(const char *s);
+extern char *arch_parse_call_target(char *t);
+
 #ifdef HAVE_ANNOTATE_INS_SUPPORT
 #include <annotate_ins.h>
 #else
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/7] perf annotate: Architecture neutral handling of return instruction
  2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
                   ` (3 preceding siblings ...)
  2016-05-19 16:59 ` [PATCH 4/7] perf annotate: Separate out architecture specific parsing Chris Ryder
@ 2016-05-19 16:59 ` Chris Ryder
  2016-05-19 16:59 ` [PATCH 6/7] perf annotate: Make action message be architecture specific Chris Ryder
  2016-05-19 16:59 ` [PATCH 7/7] perf annotate: AArch64 support Chris Ryder
  6 siblings, 0 replies; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

Currently perf annotate is hard coded to look for the x86 'retq'
instruction when annotating disassembly, regardless of the target
architecture. Move architecture specific processing of return
instructions into per-architecture header files.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 tools/perf/arch/arm/util/annotate_ins.c |  7 +++++++
 tools/perf/arch/x86/util/annotate_ins.c |  5 +++++
 tools/perf/ui/browsers/annotate.c       | 13 +++++++------
 tools/perf/util/annotate_ins.c          |  5 +++++
 tools/perf/util/annotate_ins.h          |  3 +++
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm/util/annotate_ins.c b/tools/perf/arch/arm/util/annotate_ins.c
index 87fc691..7ed4603 100644
--- a/tools/perf/arch/arm/util/annotate_ins.c
+++ b/tools/perf/arch/arm/util/annotate_ins.c
@@ -1,6 +1,13 @@
 #include <string.h>
 #include <util/annotate_ins.h>
 
+#include <linux/compiler.h>
+
+bool arch_is_return_ins(const char *s __maybe_unused)
+{
+	return false;
+}
+
 char *arch_parse_mov_comment(const char *s)
 {
 	return strchr(s, ';');
diff --git a/tools/perf/arch/x86/util/annotate_ins.c b/tools/perf/arch/x86/util/annotate_ins.c
index b007cd3..58aa4fa 100644
--- a/tools/perf/arch/x86/util/annotate_ins.c
+++ b/tools/perf/arch/x86/util/annotate_ins.c
@@ -1,6 +1,11 @@
 #include <string.h>
 #include <util/annotate_ins.h>
 
+bool arch_is_return_ins(const char *s)
+{
+	return !strcmp(s, "retq");
+}
+
 char *arch_parse_mov_comment(const char *s)
 {
 	return strchr(s, '#');
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 4fc208e..6816faf 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -8,6 +8,7 @@
 #include "../../util/sort.h"
 #include "../../util/symbol.h"
 #include "../../util/evsel.h"
+#include "../../util/annotate_ins.h"
 #include <pthread.h>
 
 struct disasm_line_samples {
@@ -226,11 +227,11 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 				ui_browser__write_nstring(browser, " ", 2);
 			}
 		} else {
-			if (strcmp(dl->name, "retq")) {
-				ui_browser__write_nstring(browser, " ", 2);
-			} else {
+			if (arch_is_return_ins(dl->name)) {
 				ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
 				SLsmg_write_char(' ');
+			} else {
+				ui_browser__write_nstring(browser, " ", 2);
 			}
 		}
 
@@ -843,9 +844,9 @@ show_help:
 			else if (browser->selection->offset == -1)
 				ui_helpline__puts("Actions are only available for assembly lines.");
 			else if (!browser->selection->ins) {
-				if (strcmp(browser->selection->name, "retq"))
-					goto show_sup_ins;
-				goto out;
+				if (arch_is_return_ins(browser->selection->name))
+					goto out;
+				goto show_sup_ins;
 			} else if (!(annotate_browser__jump(browser) ||
 				     annotate_browser__callq(browser, evsel, hbt))) {
 show_sup_ins:
diff --git a/tools/perf/util/annotate_ins.c b/tools/perf/util/annotate_ins.c
index 3867545..bb8fd01 100644
--- a/tools/perf/util/annotate_ins.c
+++ b/tools/perf/util/annotate_ins.c
@@ -3,6 +3,11 @@
 #include <linux/compiler.h>
 #include <util/annotate_ins.h>
 
+bool arch_is_return_ins(const char *s __maybe_unused)
+{
+	return false;
+}
+
 char *arch_parse_mov_comment(const char *s __maybe_unused)
 {
 	return NULL;
diff --git a/tools/perf/util/annotate_ins.h b/tools/perf/util/annotate_ins.h
index a80f493..15ac482 100644
--- a/tools/perf/util/annotate_ins.h
+++ b/tools/perf/util/annotate_ins.h
@@ -1,6 +1,9 @@
 #ifndef __ANNOTATE_INS_H
 #define __ANNOTATE_INS_H
 
+#include <linux/types.h>
+
+extern bool arch_is_return_ins(const char *s);
 extern char *arch_parse_mov_comment(const char *s);
 extern char *arch_parse_call_target(char *t);
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/7] perf annotate: Make action message be architecture specific
  2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
                   ` (4 preceding siblings ...)
  2016-05-19 16:59 ` [PATCH 5/7] perf annotate: Architecture neutral handling of return instruction Chris Ryder
@ 2016-05-19 16:59 ` Chris Ryder
  2016-05-19 16:59 ` [PATCH 7/7] perf annotate: AArch64 support Chris Ryder
  6 siblings, 0 replies; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

The help message displayed by perf report when pressing right arrow
or enter keys on an instruction that doesn't have an action is
currently hard coded and displays x86 instructions regardless of
the target architecture. Make the help message architecture specific.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 tools/perf/arch/arm/include/annotate_ins.h | 2 ++
 tools/perf/arch/x86/include/annotate_ins.h | 2 ++
 tools/perf/ui/browsers/annotate.c          | 2 +-
 tools/perf/util/annotate_ins.h             | 1 +
 4 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/arm/include/annotate_ins.h b/tools/perf/arch/arm/include/annotate_ins.h
index e8ea98d..143c478 100644
--- a/tools/perf/arch/arm/include/annotate_ins.h
+++ b/tools/perf/arch/arm/include/annotate_ins.h
@@ -22,4 +22,6 @@
 	{ .name = "orr",   .ops  = &mov_ops, }, \
 	}
 
+#define ARCH_ACTIONS "Actions are only available for branch instructions."
+
 #endif /* ARCH_ANNOTATE_INS_H */
diff --git a/tools/perf/arch/x86/include/annotate_ins.h b/tools/perf/arch/x86/include/annotate_ins.h
index 179b50e..2e889b8 100644
--- a/tools/perf/arch/x86/include/annotate_ins.h
+++ b/tools/perf/arch/x86/include/annotate_ins.h
@@ -79,4 +79,6 @@
 	{ .name = "xbeginq", .ops  = &jump_ops, }, \
 	}
 
+#define ARCH_ACTIONS "Actions are only available for 'callq', 'retq' & jump instructions."
+
 #endif /* ARCH_ANNOTATE_INS_H */
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 6816faf..56064f6 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -850,7 +850,7 @@ show_help:
 			} else if (!(annotate_browser__jump(browser) ||
 				     annotate_browser__callq(browser, evsel, hbt))) {
 show_sup_ins:
-				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
+				ui_helpline__puts(ARCH_ACTIONS);
 			}
 			continue;
 		case 't':
diff --git a/tools/perf/util/annotate_ins.h b/tools/perf/util/annotate_ins.h
index 15ac482..27457f8f 100644
--- a/tools/perf/util/annotate_ins.h
+++ b/tools/perf/util/annotate_ins.h
@@ -11,6 +11,7 @@ extern char *arch_parse_call_target(char *t);
 #include <annotate_ins.h>
 #else
 #define ARCH_INSTRUCTIONS { }
+#define ARCH_ACTIONS "Actions are not supported on this architecture"
 #endif
 
 #endif /* __ANNOTATE_INS_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/7] perf annotate: AArch64 support
  2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
                   ` (5 preceding siblings ...)
  2016-05-19 16:59 ` [PATCH 6/7] perf annotate: Make action message be architecture specific Chris Ryder
@ 2016-05-19 16:59 ` Chris Ryder
  6 siblings, 0 replies; 12+ messages in thread
From: Chris Ryder @ 2016-05-19 16:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Chris Ryder, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, linux-perf-users,
	Will Deacon, Mark Rutland

Add basic support to recognise AArch64 assembly. This allows perf to
identify AArch64 instructions that branch to other parts within the
same function, thereby properly annotating them.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 tools/perf/arch/arm64/include/annotate_ins.h | 40 ++++++++++++++++++++++++++++
 tools/perf/arch/arm64/util/Build             |  2 ++
 tools/perf/arch/arm64/util/annotate_ins.c    | 21 +++++++++++++++
 tools/perf/config/Makefile                   |  1 +
 4 files changed, 64 insertions(+)
 create mode 100644 tools/perf/arch/arm64/include/annotate_ins.h
 create mode 100644 tools/perf/arch/arm64/util/annotate_ins.c

diff --git a/tools/perf/arch/arm64/include/annotate_ins.h b/tools/perf/arch/arm64/include/annotate_ins.h
new file mode 100644
index 0000000..50a5771
--- /dev/null
+++ b/tools/perf/arch/arm64/include/annotate_ins.h
@@ -0,0 +1,40 @@
+#ifndef ARCH_ANNOTATE_INS_H
+#define ARCH_ANNOTATE_INS_H
+
+#define ARCH_INSTRUCTIONS { \
+	{ .name = "add",   .ops  = &mov_ops, }, \
+	{ .name = "and",   .ops  = &mov_ops, }, \
+	{ .name = "b",     .ops  = &jump_ops, }, /* might also be a call */ \
+	{ .name = "b.al",  .ops  = &jump_ops, }, \
+	{ .name = "b.cc",  .ops  = &jump_ops, }, \
+	{ .name = "b.cs",  .ops  = &jump_ops, }, \
+	{ .name = "b.eq",  .ops  = &jump_ops, }, \
+	{ .name = "b.ge",  .ops  = &jump_ops, }, \
+	{ .name = "b.gt",  .ops  = &jump_ops, }, \
+	{ .name = "b.hi",  .ops  = &jump_ops, }, \
+	{ .name = "b.hs",  .ops  = &jump_ops, }, \
+	{ .name = "b.le",  .ops  = &jump_ops, }, \
+	{ .name = "b.lo",  .ops  = &jump_ops, }, \
+	{ .name = "b.ls",  .ops  = &jump_ops, }, \
+	{ .name = "b.lt",  .ops  = &jump_ops, }, \
+	{ .name = "b.mi",  .ops  = &jump_ops, }, \
+	{ .name = "b.ne",  .ops  = &jump_ops, }, \
+	{ .name = "b.nv",  .ops  = &jump_ops, }, \
+	{ .name = "b.pl",  .ops  = &jump_ops, }, \
+	{ .name = "b.vc",  .ops  = &jump_ops, }, \
+	{ .name = "b.vs",  .ops  = &jump_ops, }, \
+	{ .name = "bl",    .ops  = &call_ops, }, \
+	{ .name = "blr",   .ops  = &call_ops, }, \
+	{ .name = "cbnz",  .ops  = &jump_ops, }, \
+	{ .name = "cbz",   .ops  = &jump_ops, }, \
+	{ .name = "cmp",   .ops  = &mov_ops, }, \
+	{ .name = "mov",   .ops  = &mov_ops, }, \
+	{ .name = "nop",   .ops  = &nop_ops, }, \
+	{ .name = "orr",   .ops  = &mov_ops, }, \
+	{ .name = "tbnz",  .ops  = &jump_ops, }, \
+	{ .name = "tbz",   .ops  = &jump_ops, }, \
+	}
+
+#define ARCH_ACTIONS "Actions are only available for 'ret' & branch instructions."
+
+#endif /* ARCH_ANNOTATE_INS_H */
diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index e58123a8..10c78ba 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -1,2 +1,4 @@
+libperf-y     += annotate_ins.o
+
 libperf-$(CONFIG_DWARF)     += dwarf-regs.o
 libperf-$(CONFIG_LIBUNWIND) += unwind-libunwind.o
diff --git a/tools/perf/arch/arm64/util/annotate_ins.c b/tools/perf/arch/arm64/util/annotate_ins.c
new file mode 100644
index 0000000..eba36b4
--- /dev/null
+++ b/tools/perf/arch/arm64/util/annotate_ins.c
@@ -0,0 +1,21 @@
+#include <string.h>
+#include <linux/compiler.h>
+#include <util/annotate_ins.h>
+
+bool arch_is_return_ins(const char *s __maybe_unused)
+{
+	return !strcmp(s, "ret");
+}
+
+char *arch_parse_mov_comment(const char *s)
+{
+	return strchr(s, ';');
+}
+
+char *arch_parse_call_target(char *t)
+{
+	if (strchr(t, '+'))
+		return NULL;
+
+	return t;
+}
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index d3eba89..47b26c9 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -47,6 +47,7 @@ endif
 
 ifeq ($(ARCH),arm64)
   NO_PERF_REGS := 0
+  NO_ANNOTATE_INS := 0
   LIBUNWIND_LIBS = -lunwind -lunwind-aarch64
 endif
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/7] pref annotate: Separate architecture specific annotation support
  2016-05-19 16:59 ` [PATCH 3/7] pref annotate: Separate architecture specific annotation support Chris Ryder
@ 2016-05-19 19:45   ` Arnaldo Carvalho de Melo
  2016-05-20  9:44     ` Chris Ryder
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-05-19 19:45 UTC (permalink / raw)
  To: Chris Ryder
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	linux-perf-users, Will Deacon, Mark Rutland

Em Thu, May 19, 2016 at 05:59:47PM +0100, Chris Ryder escreveu:
> Currently the list of instructions recognised by perf annotate
> contains an intermingled mix of x86 and ARM instructions, and the x86
> instructions are unconditionally included on all platforms. This means
> that perf attempts to parse x86 instructions on non-x86 platforms.
> Refactor the instruction list into per-architecture header files so that
> only the instructions applicable to the target architecture are parsed.

How about annotating a perf.data file collected on ARM on a x86_64
machine?

I think we should have this keyed by the arch name and have all the
tables available, using the one we get from the perf.data header.

Look at the "[PATCH v4 2/6] perf tools: Promote proper messages for
cross-platform unwind " patch, it has the bits you need to get the arch
from the perf.data file header (session->machines->machine->env->arch)

I applied the first patch.

- Arnaldo
 
> Signed-off-by: Chris Ryder <chris.ryder@arm.com>
> Acked-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: linux-perf-users@vger.kernel.org
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  tools/perf/arch/arm/include/annotate_ins.h |  25 +++++++
>  tools/perf/arch/x86/include/annotate_ins.h |  82 ++++++++++++++++++++++
>  tools/perf/config/Makefile                 |  11 +++
>  tools/perf/util/annotate.c                 | 105 +++--------------------------
>  tools/perf/util/annotate_ins.h             |  10 +++
>  5 files changed, 136 insertions(+), 97 deletions(-)
>  create mode 100644 tools/perf/arch/arm/include/annotate_ins.h
>  create mode 100644 tools/perf/arch/x86/include/annotate_ins.h
>  create mode 100644 tools/perf/util/annotate_ins.h
> 
> diff --git a/tools/perf/arch/arm/include/annotate_ins.h b/tools/perf/arch/arm/include/annotate_ins.h
> new file mode 100644
> index 0000000..e8ea98d
> --- /dev/null
> +++ b/tools/perf/arch/arm/include/annotate_ins.h
> @@ -0,0 +1,25 @@
> +#ifndef ARCH_ANNOTATE_INS_H
> +#define ARCH_ANNOTATE_INS_H
> +
> +#define ARCH_INSTRUCTIONS { \
> +	{ .name = "add",   .ops  = &mov_ops, }, \
> +	{ .name = "and",   .ops  = &mov_ops, }, \
> +	{ .name = "b",     .ops  = &jump_ops, }, /* might also be a call */ \
> +	{ .name = "bcc",   .ops  = &jump_ops, }, \
> +	{ .name = "bcs",   .ops  = &jump_ops, }, \
> +	{ .name = "beq",   .ops  = &jump_ops, }, \
> +	{ .name = "bge",   .ops  = &jump_ops, }, \
> +	{ .name = "bgt",   .ops  = &jump_ops, }, \
> +	{ .name = "bhi",   .ops  = &jump_ops, }, \
> +	{ .name = "bl",    .ops  = &call_ops, }, \
> +	{ .name = "bls",   .ops  = &jump_ops, }, \
> +	{ .name = "blt",   .ops  = &jump_ops, }, \
> +	{ .name = "blx",   .ops  = &call_ops, }, \
> +	{ .name = "bne",   .ops  = &jump_ops, }, \
> +	{ .name = "cmp",   .ops  = &mov_ops, }, \
> +	{ .name = "mov",   .ops  = &mov_ops, }, \
> +	{ .name = "nop",   .ops  = &nop_ops, }, \
> +	{ .name = "orr",   .ops  = &mov_ops, }, \
> +	}
> +
> +#endif /* ARCH_ANNOTATE_INS_H */
> diff --git a/tools/perf/arch/x86/include/annotate_ins.h b/tools/perf/arch/x86/include/annotate_ins.h
> new file mode 100644
> index 0000000..179b50e
> --- /dev/null
> +++ b/tools/perf/arch/x86/include/annotate_ins.h
> @@ -0,0 +1,82 @@
> +#ifndef ARCH_ANNOTATE_INS_H
> +#define ARCH_ANNOTATE_INS_H
> +
> +#define ARCH_INSTRUCTIONS { \
> +	{ .name = "add",     .ops  = &mov_ops, }, \
> +	{ .name = "addl",    .ops  = &mov_ops, }, \
> +	{ .name = "addq",    .ops  = &mov_ops, }, \
> +	{ .name = "addw",    .ops  = &mov_ops, }, \
> +	{ .name = "and",     .ops  = &mov_ops, }, \
> +	{ .name = "bts",     .ops  = &mov_ops, }, \
> +	{ .name = "call",    .ops  = &call_ops, }, \
> +	{ .name = "callq",   .ops  = &call_ops, }, \
> +	{ .name = "cmp",     .ops  = &mov_ops, }, \
> +	{ .name = "cmpb",    .ops  = &mov_ops, }, \
> +	{ .name = "cmpl",    .ops  = &mov_ops, }, \
> +	{ .name = "cmpq",    .ops  = &mov_ops, }, \
> +	{ .name = "cmpw",    .ops  = &mov_ops, }, \
> +	{ .name = "cmpxch",  .ops  = &mov_ops, }, \
> +	{ .name = "dec",     .ops  = &dec_ops, }, \
> +	{ .name = "decl",    .ops  = &dec_ops, }, \
> +	{ .name = "imul",    .ops  = &mov_ops, }, \
> +	{ .name = "inc",     .ops  = &dec_ops, }, \
> +	{ .name = "incl",    .ops  = &dec_ops, }, \
> +	{ .name = "ja",	     .ops  = &jump_ops, }, \
> +	{ .name = "jae",     .ops  = &jump_ops, }, \
> +	{ .name = "jb",	     .ops  = &jump_ops, }, \
> +	{ .name = "jbe",     .ops  = &jump_ops, }, \
> +	{ .name = "jc",	     .ops  = &jump_ops, }, \
> +	{ .name = "jcxz",    .ops  = &jump_ops, }, \
> +	{ .name = "je",	     .ops  = &jump_ops, }, \
> +	{ .name = "jecxz",   .ops  = &jump_ops, }, \
> +	{ .name = "jg",	     .ops  = &jump_ops, }, \
> +	{ .name = "jge",     .ops  = &jump_ops, }, \
> +	{ .name = "jl",      .ops  = &jump_ops, }, \
> +	{ .name = "jle",     .ops  = &jump_ops, }, \
> +	{ .name = "jmp",     .ops  = &jump_ops, }, \
> +	{ .name = "jmpq",    .ops  = &jump_ops, }, \
> +	{ .name = "jna",     .ops  = &jump_ops, }, \
> +	{ .name = "jnae",    .ops  = &jump_ops, }, \
> +	{ .name = "jnb",     .ops  = &jump_ops, }, \
> +	{ .name = "jnbe",    .ops  = &jump_ops, }, \
> +	{ .name = "jnc",     .ops  = &jump_ops, }, \
> +	{ .name = "jne",     .ops  = &jump_ops, }, \
> +	{ .name = "jng",     .ops  = &jump_ops, }, \
> +	{ .name = "jnge",    .ops  = &jump_ops, }, \
> +	{ .name = "jnl",     .ops  = &jump_ops, }, \
> +	{ .name = "jnle",    .ops  = &jump_ops, }, \
> +	{ .name = "jno",     .ops  = &jump_ops, }, \
> +	{ .name = "jnp",     .ops  = &jump_ops, }, \
> +	{ .name = "jns",     .ops  = &jump_ops, }, \
> +	{ .name = "jnz",     .ops  = &jump_ops, }, \
> +	{ .name = "jo",	     .ops  = &jump_ops, }, \
> +	{ .name = "jp",	     .ops  = &jump_ops, }, \
> +	{ .name = "jpe",     .ops  = &jump_ops, }, \
> +	{ .name = "jpo",     .ops  = &jump_ops, }, \
> +	{ .name = "jrcxz",   .ops  = &jump_ops, }, \
> +	{ .name = "js",	     .ops  = &jump_ops, }, \
> +	{ .name = "jz",	     .ops  = &jump_ops, }, \
> +	{ .name = "lea",     .ops  = &mov_ops, }, \
> +	{ .name = "lock",    .ops  = &lock_ops, }, \
> +	{ .name = "mov",     .ops  = &mov_ops, }, \
> +	{ .name = "movb",    .ops  = &mov_ops, }, \
> +	{ .name = "movdqa",  .ops  = &mov_ops, }, \
> +	{ .name = "movl",    .ops  = &mov_ops, }, \
> +	{ .name = "movq",    .ops  = &mov_ops, }, \
> +	{ .name = "movslq",  .ops  = &mov_ops, }, \
> +	{ .name = "movzbl",  .ops  = &mov_ops, }, \
> +	{ .name = "movzwl",  .ops  = &mov_ops, }, \
> +	{ .name = "nop",     .ops  = &nop_ops, }, \
> +	{ .name = "nopl",    .ops  = &nop_ops, }, \
> +	{ .name = "nopw",    .ops  = &nop_ops, }, \
> +	{ .name = "or",      .ops  = &mov_ops, }, \
> +	{ .name = "orl",     .ops  = &mov_ops, }, \
> +	{ .name = "test",    .ops  = &mov_ops, }, \
> +	{ .name = "testb",   .ops  = &mov_ops, }, \
> +	{ .name = "testl",   .ops  = &mov_ops, }, \
> +	{ .name = "xadd",    .ops  = &mov_ops, }, \
> +	{ .name = "xbeginl", .ops  = &jump_ops, }, \
> +	{ .name = "xbeginq", .ops  = &jump_ops, }, \
> +	}
> +
> +#endif /* ARCH_ANNOTATE_INS_H */
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index 1e46277..d3eba89 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -22,6 +22,7 @@ include $(srctree)/tools/scripts/Makefile.arch
>  $(call detected_var,ARCH)
>  
>  NO_PERF_REGS := 1
> +NO_ANNOTATE_INS := 1
>  
>  # Additional ARCH settings for x86
>  ifeq ($(ARCH),x86)
> @@ -35,10 +36,12 @@ ifeq ($(ARCH),x86)
>      LIBUNWIND_LIBS = -lunwind-x86 -llzma -lunwind
>    endif
>    NO_PERF_REGS := 0
> +  NO_ANNOTATE_INS := 0
>  endif
>  
>  ifeq ($(ARCH),arm)
>    NO_PERF_REGS := 0
> +  NO_ANNOTATE_INS := 0
>    LIBUNWIND_LIBS = -lunwind -lunwind-arm
>  endif
>  
> @@ -51,6 +54,10 @@ ifeq ($(NO_PERF_REGS),0)
>    $(call detected,CONFIG_PERF_REGS)
>  endif
>  
> +ifeq ($(NO_ANNOTATE_INS),0)
> +  $(call detected,CONFIG_ANNOTATE_INS)
> +endif
> +
>  # So far there's only x86 and arm libdw unwind support merged in perf.
>  # Disable it on all other architectures in case libdw unwind
>  # support is detected in system. Add supported architectures
> @@ -83,6 +90,10 @@ ifeq ($(NO_PERF_REGS),0)
>    CFLAGS += -DHAVE_PERF_REGS_SUPPORT
>  endif
>  
> +ifeq ($(NO_ANNOTATE_INS),0)
> +  CFLAGS += -DHAVE_ANNOTATE_INS_SUPPORT
> +endif
> +
>  # for linking with debug library, run like:
>  # make DEBUG=1 LIBDW_DIR=/opt/libdw/
>  ifdef LIBDW_DIR
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 619bc27..71c1dd5 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -20,6 +20,7 @@
>  #include <regex.h>
>  #include <pthread.h>
>  #include <linux/bitops.h>
> +#include "annotate_ins.h"
>  
>  const char 	*disassembler_style;
>  const char	*objdump_path;
> @@ -107,7 +108,7 @@ static int call__scnprintf(struct ins *ins, char *bf, size_t size,
>  	return scnprintf(bf, size, "%-6.6s *%" PRIx64, ins->name, ops->target.addr);
>  }
>  
> -static struct ins_ops call_ops = {
> +static struct ins_ops call_ops __maybe_unused = {
>  	.parse	   = call__parse,
>  	.scnprintf = call__scnprintf,
>  };
> @@ -137,7 +138,7 @@ static int jump__scnprintf(struct ins *ins, char *bf, size_t size,
>  	return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, ops->target.offset);
>  }
>  
> -static struct ins_ops jump_ops = {
> +static struct ins_ops jump_ops __maybe_unused = {
>  	.parse	   = jump__parse,
>  	.scnprintf = jump__scnprintf,
>  };
> @@ -230,7 +231,7 @@ static void lock__delete(struct ins_operands *ops)
>  	zfree(&ops->target.name);
>  }
>  
> -static struct ins_ops lock_ops = {
> +static struct ins_ops lock_ops __maybe_unused = {
>  	.free	   = lock__delete,
>  	.parse	   = lock__parse,
>  	.scnprintf = lock__scnprintf,
> @@ -298,7 +299,7 @@ static int mov__scnprintf(struct ins *ins, char *bf, size_t size,
>  			 ops->target.name ?: ops->target.raw);
>  }
>  
> -static struct ins_ops mov_ops = {
> +static struct ins_ops mov_ops __maybe_unused = {
>  	.parse	   = mov__parse,
>  	.scnprintf = mov__scnprintf,
>  };
> @@ -339,7 +340,7 @@ static int dec__scnprintf(struct ins *ins, char *bf, size_t size,
>  			 ops->target.name ?: ops->target.raw);
>  }
>  
> -static struct ins_ops dec_ops = {
> +static struct ins_ops dec_ops __maybe_unused = {
>  	.parse	   = dec__parse,
>  	.scnprintf = dec__scnprintf,
>  };
> @@ -350,101 +351,11 @@ static int nop__scnprintf(struct ins *ins __maybe_unused, char *bf, size_t size,
>  	return scnprintf(bf, size, "%-6.6s", "nop");
>  }
>  
> -static struct ins_ops nop_ops = {
> +static struct ins_ops nop_ops __maybe_unused = {
>  	.scnprintf = nop__scnprintf,
>  };
>  
> -static struct ins instructions[] = {
> -	{ .name = "add",   .ops  = &mov_ops, },
> -	{ .name = "addl",  .ops  = &mov_ops, },
> -	{ .name = "addq",  .ops  = &mov_ops, },
> -	{ .name = "addw",  .ops  = &mov_ops, },
> -	{ .name = "and",   .ops  = &mov_ops, },
> -#ifdef __arm__
> -	{ .name = "b",     .ops  = &jump_ops, }, // might also be a call
> -	{ .name = "bcc",   .ops  = &jump_ops, },
> -	{ .name = "bcs",   .ops  = &jump_ops, },
> -	{ .name = "beq",   .ops  = &jump_ops, },
> -	{ .name = "bge",   .ops  = &jump_ops, },
> -	{ .name = "bgt",   .ops  = &jump_ops, },
> -	{ .name = "bhi",   .ops  = &jump_ops, },
> -	{ .name = "bl",    .ops  = &call_ops, },
> -	{ .name = "bls",   .ops  = &jump_ops, },
> -	{ .name = "blt",   .ops  = &jump_ops, },
> -	{ .name = "blx",   .ops  = &call_ops, },
> -	{ .name = "bne",   .ops  = &jump_ops, },
> -#endif
> -	{ .name = "bts",   .ops  = &mov_ops, },
> -	{ .name = "call",  .ops  = &call_ops, },
> -	{ .name = "callq", .ops  = &call_ops, },
> -	{ .name = "cmp",   .ops  = &mov_ops, },
> -	{ .name = "cmpb",  .ops  = &mov_ops, },
> -	{ .name = "cmpl",  .ops  = &mov_ops, },
> -	{ .name = "cmpq",  .ops  = &mov_ops, },
> -	{ .name = "cmpw",  .ops  = &mov_ops, },
> -	{ .name = "cmpxch", .ops  = &mov_ops, },
> -	{ .name = "dec",   .ops  = &dec_ops, },
> -	{ .name = "decl",  .ops  = &dec_ops, },
> -	{ .name = "imul",  .ops  = &mov_ops, },
> -	{ .name = "inc",   .ops  = &dec_ops, },
> -	{ .name = "incl",  .ops  = &dec_ops, },
> -	{ .name = "ja",	   .ops  = &jump_ops, },
> -	{ .name = "jae",   .ops  = &jump_ops, },
> -	{ .name = "jb",	   .ops  = &jump_ops, },
> -	{ .name = "jbe",   .ops  = &jump_ops, },
> -	{ .name = "jc",	   .ops  = &jump_ops, },
> -	{ .name = "jcxz",  .ops  = &jump_ops, },
> -	{ .name = "je",	   .ops  = &jump_ops, },
> -	{ .name = "jecxz", .ops  = &jump_ops, },
> -	{ .name = "jg",	   .ops  = &jump_ops, },
> -	{ .name = "jge",   .ops  = &jump_ops, },
> -	{ .name = "jl",    .ops  = &jump_ops, },
> -	{ .name = "jle",   .ops  = &jump_ops, },
> -	{ .name = "jmp",   .ops  = &jump_ops, },
> -	{ .name = "jmpq",  .ops  = &jump_ops, },
> -	{ .name = "jna",   .ops  = &jump_ops, },
> -	{ .name = "jnae",  .ops  = &jump_ops, },
> -	{ .name = "jnb",   .ops  = &jump_ops, },
> -	{ .name = "jnbe",  .ops  = &jump_ops, },
> -	{ .name = "jnc",   .ops  = &jump_ops, },
> -	{ .name = "jne",   .ops  = &jump_ops, },
> -	{ .name = "jng",   .ops  = &jump_ops, },
> -	{ .name = "jnge",  .ops  = &jump_ops, },
> -	{ .name = "jnl",   .ops  = &jump_ops, },
> -	{ .name = "jnle",  .ops  = &jump_ops, },
> -	{ .name = "jno",   .ops  = &jump_ops, },
> -	{ .name = "jnp",   .ops  = &jump_ops, },
> -	{ .name = "jns",   .ops  = &jump_ops, },
> -	{ .name = "jnz",   .ops  = &jump_ops, },
> -	{ .name = "jo",	   .ops  = &jump_ops, },
> -	{ .name = "jp",	   .ops  = &jump_ops, },
> -	{ .name = "jpe",   .ops  = &jump_ops, },
> -	{ .name = "jpo",   .ops  = &jump_ops, },
> -	{ .name = "jrcxz", .ops  = &jump_ops, },
> -	{ .name = "js",	   .ops  = &jump_ops, },
> -	{ .name = "jz",	   .ops  = &jump_ops, },
> -	{ .name = "lea",   .ops  = &mov_ops, },
> -	{ .name = "lock",  .ops  = &lock_ops, },
> -	{ .name = "mov",   .ops  = &mov_ops, },
> -	{ .name = "movb",  .ops  = &mov_ops, },
> -	{ .name = "movdqa",.ops  = &mov_ops, },
> -	{ .name = "movl",  .ops  = &mov_ops, },
> -	{ .name = "movq",  .ops  = &mov_ops, },
> -	{ .name = "movslq", .ops  = &mov_ops, },
> -	{ .name = "movzbl", .ops  = &mov_ops, },
> -	{ .name = "movzwl", .ops  = &mov_ops, },
> -	{ .name = "nop",   .ops  = &nop_ops, },
> -	{ .name = "nopl",  .ops  = &nop_ops, },
> -	{ .name = "nopw",  .ops  = &nop_ops, },
> -	{ .name = "or",    .ops  = &mov_ops, },
> -	{ .name = "orl",   .ops  = &mov_ops, },
> -	{ .name = "test",  .ops  = &mov_ops, },
> -	{ .name = "testb", .ops  = &mov_ops, },
> -	{ .name = "testl", .ops  = &mov_ops, },
> -	{ .name = "xadd",  .ops  = &mov_ops, },
> -	{ .name = "xbeginl", .ops  = &jump_ops, },
> -	{ .name = "xbeginq", .ops  = &jump_ops, },
> -};
> +static struct ins instructions[] = ARCH_INSTRUCTIONS;
>  
>  static int ins__key_cmp(const void *name, const void *insp)
>  {
> diff --git a/tools/perf/util/annotate_ins.h b/tools/perf/util/annotate_ins.h
> new file mode 100644
> index 0000000..2ec9a05
> --- /dev/null
> +++ b/tools/perf/util/annotate_ins.h
> @@ -0,0 +1,10 @@
> +#ifndef __ANNOTATE_INS_H
> +#define __ANNOTATE_INS_H
> +
> +#ifdef HAVE_ANNOTATE_INS_SUPPORT
> +#include <annotate_ins.h>
> +#else
> +#define ARCH_INSTRUCTIONS { }
> +#endif
> +
> +#endif /* __ANNOTATE_INS_H */
> -- 
> 2.1.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/7] pref annotate: Separate architecture specific annotation support
  2016-05-19 19:45   ` Arnaldo Carvalho de Melo
@ 2016-05-20  9:44     ` Chris Ryder
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Ryder @ 2016-05-20  9:44 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	linux-perf-users, Will Deacon, Mark Rutland

On Thu, May 19, 2016 at 04:45:40PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, May 19, 2016 at 05:59:47PM +0100, Chris Ryder escreveu:
> > Currently the list of instructions recognised by perf annotate
> > contains an intermingled mix of x86 and ARM instructions, and the x86
> > instructions are unconditionally included on all platforms. This means
> > that perf attempts to parse x86 instructions on non-x86 platforms.
> > Refactor the instruction list into per-architecture header files so that
> > only the instructions applicable to the target architecture are parsed.
> 
> How about annotating a perf.data file collected on ARM on a x86_64
> machine?
> 
> I think we should have this keyed by the arch name and have all the
> tables available, using the one we get from the perf.data header.

Yes, that would be a better way to go - I'll take a look at doing it
that way.

> 
> Look at the "[PATCH v4 2/6] perf tools: Promote proper messages for
> cross-platform unwind " patch, it has the bits you need to get the arch
> from the perf.data file header (session->machines->machine->env->arch)

Thanks for pointing me in the right direction,
Chris.

> 
> I applied the first patch.
> 
> - Arnaldo
>  
> > Signed-off-by: Chris Ryder <chris.ryder@arm.com>
> > Acked-by: Pawel Moll <pawel.moll@arm.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: linux-perf-users@vger.kernel.org
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  tools/perf/arch/arm/include/annotate_ins.h |  25 +++++++
> >  tools/perf/arch/x86/include/annotate_ins.h |  82 ++++++++++++++++++++++
> >  tools/perf/config/Makefile                 |  11 +++
> >  tools/perf/util/annotate.c                 | 105 +++--------------------------
> >  tools/perf/util/annotate_ins.h             |  10 +++
> >  5 files changed, 136 insertions(+), 97 deletions(-)
> >  create mode 100644 tools/perf/arch/arm/include/annotate_ins.h
> >  create mode 100644 tools/perf/arch/x86/include/annotate_ins.h
> >  create mode 100644 tools/perf/util/annotate_ins.h
> > 
> > diff --git a/tools/perf/arch/arm/include/annotate_ins.h b/tools/perf/arch/arm/include/annotate_ins.h
> > new file mode 100644
> > index 0000000..e8ea98d
> > --- /dev/null
> > +++ b/tools/perf/arch/arm/include/annotate_ins.h
> > @@ -0,0 +1,25 @@
> > +#ifndef ARCH_ANNOTATE_INS_H
> > +#define ARCH_ANNOTATE_INS_H
> > +
> > +#define ARCH_INSTRUCTIONS { \
> > +	{ .name = "add",   .ops  = &mov_ops, }, \
> > +	{ .name = "and",   .ops  = &mov_ops, }, \
> > +	{ .name = "b",     .ops  = &jump_ops, }, /* might also be a call */ \
> > +	{ .name = "bcc",   .ops  = &jump_ops, }, \
> > +	{ .name = "bcs",   .ops  = &jump_ops, }, \
> > +	{ .name = "beq",   .ops  = &jump_ops, }, \
> > +	{ .name = "bge",   .ops  = &jump_ops, }, \
> > +	{ .name = "bgt",   .ops  = &jump_ops, }, \
> > +	{ .name = "bhi",   .ops  = &jump_ops, }, \
> > +	{ .name = "bl",    .ops  = &call_ops, }, \
> > +	{ .name = "bls",   .ops  = &jump_ops, }, \
> > +	{ .name = "blt",   .ops  = &jump_ops, }, \
> > +	{ .name = "blx",   .ops  = &call_ops, }, \
> > +	{ .name = "bne",   .ops  = &jump_ops, }, \
> > +	{ .name = "cmp",   .ops  = &mov_ops, }, \
> > +	{ .name = "mov",   .ops  = &mov_ops, }, \
> > +	{ .name = "nop",   .ops  = &nop_ops, }, \
> > +	{ .name = "orr",   .ops  = &mov_ops, }, \
> > +	}
> > +
> > +#endif /* ARCH_ANNOTATE_INS_H */
> > diff --git a/tools/perf/arch/x86/include/annotate_ins.h b/tools/perf/arch/x86/include/annotate_ins.h
> > new file mode 100644
> > index 0000000..179b50e
> > --- /dev/null
> > +++ b/tools/perf/arch/x86/include/annotate_ins.h
> > @@ -0,0 +1,82 @@
> > +#ifndef ARCH_ANNOTATE_INS_H
> > +#define ARCH_ANNOTATE_INS_H
> > +
> > +#define ARCH_INSTRUCTIONS { \
> > +	{ .name = "add",     .ops  = &mov_ops, }, \
> > +	{ .name = "addl",    .ops  = &mov_ops, }, \
> > +	{ .name = "addq",    .ops  = &mov_ops, }, \
> > +	{ .name = "addw",    .ops  = &mov_ops, }, \
> > +	{ .name = "and",     .ops  = &mov_ops, }, \
> > +	{ .name = "bts",     .ops  = &mov_ops, }, \
> > +	{ .name = "call",    .ops  = &call_ops, }, \
> > +	{ .name = "callq",   .ops  = &call_ops, }, \
> > +	{ .name = "cmp",     .ops  = &mov_ops, }, \
> > +	{ .name = "cmpb",    .ops  = &mov_ops, }, \
> > +	{ .name = "cmpl",    .ops  = &mov_ops, }, \
> > +	{ .name = "cmpq",    .ops  = &mov_ops, }, \
> > +	{ .name = "cmpw",    .ops  = &mov_ops, }, \
> > +	{ .name = "cmpxch",  .ops  = &mov_ops, }, \
> > +	{ .name = "dec",     .ops  = &dec_ops, }, \
> > +	{ .name = "decl",    .ops  = &dec_ops, }, \
> > +	{ .name = "imul",    .ops  = &mov_ops, }, \
> > +	{ .name = "inc",     .ops  = &dec_ops, }, \
> > +	{ .name = "incl",    .ops  = &dec_ops, }, \
> > +	{ .name = "ja",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jae",     .ops  = &jump_ops, }, \
> > +	{ .name = "jb",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jbe",     .ops  = &jump_ops, }, \
> > +	{ .name = "jc",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jcxz",    .ops  = &jump_ops, }, \
> > +	{ .name = "je",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jecxz",   .ops  = &jump_ops, }, \
> > +	{ .name = "jg",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jge",     .ops  = &jump_ops, }, \
> > +	{ .name = "jl",      .ops  = &jump_ops, }, \
> > +	{ .name = "jle",     .ops  = &jump_ops, }, \
> > +	{ .name = "jmp",     .ops  = &jump_ops, }, \
> > +	{ .name = "jmpq",    .ops  = &jump_ops, }, \
> > +	{ .name = "jna",     .ops  = &jump_ops, }, \
> > +	{ .name = "jnae",    .ops  = &jump_ops, }, \
> > +	{ .name = "jnb",     .ops  = &jump_ops, }, \
> > +	{ .name = "jnbe",    .ops  = &jump_ops, }, \
> > +	{ .name = "jnc",     .ops  = &jump_ops, }, \
> > +	{ .name = "jne",     .ops  = &jump_ops, }, \
> > +	{ .name = "jng",     .ops  = &jump_ops, }, \
> > +	{ .name = "jnge",    .ops  = &jump_ops, }, \
> > +	{ .name = "jnl",     .ops  = &jump_ops, }, \
> > +	{ .name = "jnle",    .ops  = &jump_ops, }, \
> > +	{ .name = "jno",     .ops  = &jump_ops, }, \
> > +	{ .name = "jnp",     .ops  = &jump_ops, }, \
> > +	{ .name = "jns",     .ops  = &jump_ops, }, \
> > +	{ .name = "jnz",     .ops  = &jump_ops, }, \
> > +	{ .name = "jo",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jp",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jpe",     .ops  = &jump_ops, }, \
> > +	{ .name = "jpo",     .ops  = &jump_ops, }, \
> > +	{ .name = "jrcxz",   .ops  = &jump_ops, }, \
> > +	{ .name = "js",	     .ops  = &jump_ops, }, \
> > +	{ .name = "jz",	     .ops  = &jump_ops, }, \
> > +	{ .name = "lea",     .ops  = &mov_ops, }, \
> > +	{ .name = "lock",    .ops  = &lock_ops, }, \
> > +	{ .name = "mov",     .ops  = &mov_ops, }, \
> > +	{ .name = "movb",    .ops  = &mov_ops, }, \
> > +	{ .name = "movdqa",  .ops  = &mov_ops, }, \
> > +	{ .name = "movl",    .ops  = &mov_ops, }, \
> > +	{ .name = "movq",    .ops  = &mov_ops, }, \
> > +	{ .name = "movslq",  .ops  = &mov_ops, }, \
> > +	{ .name = "movzbl",  .ops  = &mov_ops, }, \
> > +	{ .name = "movzwl",  .ops  = &mov_ops, }, \
> > +	{ .name = "nop",     .ops  = &nop_ops, }, \
> > +	{ .name = "nopl",    .ops  = &nop_ops, }, \
> > +	{ .name = "nopw",    .ops  = &nop_ops, }, \
> > +	{ .name = "or",      .ops  = &mov_ops, }, \
> > +	{ .name = "orl",     .ops  = &mov_ops, }, \
> > +	{ .name = "test",    .ops  = &mov_ops, }, \
> > +	{ .name = "testb",   .ops  = &mov_ops, }, \
> > +	{ .name = "testl",   .ops  = &mov_ops, }, \
> > +	{ .name = "xadd",    .ops  = &mov_ops, }, \
> > +	{ .name = "xbeginl", .ops  = &jump_ops, }, \
> > +	{ .name = "xbeginq", .ops  = &jump_ops, }, \
> > +	}
> > +
> > +#endif /* ARCH_ANNOTATE_INS_H */
> > diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> > index 1e46277..d3eba89 100644
> > --- a/tools/perf/config/Makefile
> > +++ b/tools/perf/config/Makefile
> > @@ -22,6 +22,7 @@ include $(srctree)/tools/scripts/Makefile.arch
> >  $(call detected_var,ARCH)
> >  
> >  NO_PERF_REGS := 1
> > +NO_ANNOTATE_INS := 1
> >  
> >  # Additional ARCH settings for x86
> >  ifeq ($(ARCH),x86)
> > @@ -35,10 +36,12 @@ ifeq ($(ARCH),x86)
> >      LIBUNWIND_LIBS = -lunwind-x86 -llzma -lunwind
> >    endif
> >    NO_PERF_REGS := 0
> > +  NO_ANNOTATE_INS := 0
> >  endif
> >  
> >  ifeq ($(ARCH),arm)
> >    NO_PERF_REGS := 0
> > +  NO_ANNOTATE_INS := 0
> >    LIBUNWIND_LIBS = -lunwind -lunwind-arm
> >  endif
> >  
> > @@ -51,6 +54,10 @@ ifeq ($(NO_PERF_REGS),0)
> >    $(call detected,CONFIG_PERF_REGS)
> >  endif
> >  
> > +ifeq ($(NO_ANNOTATE_INS),0)
> > +  $(call detected,CONFIG_ANNOTATE_INS)
> > +endif
> > +
> >  # So far there's only x86 and arm libdw unwind support merged in perf.
> >  # Disable it on all other architectures in case libdw unwind
> >  # support is detected in system. Add supported architectures
> > @@ -83,6 +90,10 @@ ifeq ($(NO_PERF_REGS),0)
> >    CFLAGS += -DHAVE_PERF_REGS_SUPPORT
> >  endif
> >  
> > +ifeq ($(NO_ANNOTATE_INS),0)
> > +  CFLAGS += -DHAVE_ANNOTATE_INS_SUPPORT
> > +endif
> > +
> >  # for linking with debug library, run like:
> >  # make DEBUG=1 LIBDW_DIR=/opt/libdw/
> >  ifdef LIBDW_DIR
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 619bc27..71c1dd5 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -20,6 +20,7 @@
> >  #include <regex.h>
> >  #include <pthread.h>
> >  #include <linux/bitops.h>
> > +#include "annotate_ins.h"
> >  
> >  const char 	*disassembler_style;
> >  const char	*objdump_path;
> > @@ -107,7 +108,7 @@ static int call__scnprintf(struct ins *ins, char *bf, size_t size,
> >  	return scnprintf(bf, size, "%-6.6s *%" PRIx64, ins->name, ops->target.addr);
> >  }
> >  
> > -static struct ins_ops call_ops = {
> > +static struct ins_ops call_ops __maybe_unused = {
> >  	.parse	   = call__parse,
> >  	.scnprintf = call__scnprintf,
> >  };
> > @@ -137,7 +138,7 @@ static int jump__scnprintf(struct ins *ins, char *bf, size_t size,
> >  	return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, ops->target.offset);
> >  }
> >  
> > -static struct ins_ops jump_ops = {
> > +static struct ins_ops jump_ops __maybe_unused = {
> >  	.parse	   = jump__parse,
> >  	.scnprintf = jump__scnprintf,
> >  };
> > @@ -230,7 +231,7 @@ static void lock__delete(struct ins_operands *ops)
> >  	zfree(&ops->target.name);
> >  }
> >  
> > -static struct ins_ops lock_ops = {
> > +static struct ins_ops lock_ops __maybe_unused = {
> >  	.free	   = lock__delete,
> >  	.parse	   = lock__parse,
> >  	.scnprintf = lock__scnprintf,
> > @@ -298,7 +299,7 @@ static int mov__scnprintf(struct ins *ins, char *bf, size_t size,
> >  			 ops->target.name ?: ops->target.raw);
> >  }
> >  
> > -static struct ins_ops mov_ops = {
> > +static struct ins_ops mov_ops __maybe_unused = {
> >  	.parse	   = mov__parse,
> >  	.scnprintf = mov__scnprintf,
> >  };
> > @@ -339,7 +340,7 @@ static int dec__scnprintf(struct ins *ins, char *bf, size_t size,
> >  			 ops->target.name ?: ops->target.raw);
> >  }
> >  
> > -static struct ins_ops dec_ops = {
> > +static struct ins_ops dec_ops __maybe_unused = {
> >  	.parse	   = dec__parse,
> >  	.scnprintf = dec__scnprintf,
> >  };
> > @@ -350,101 +351,11 @@ static int nop__scnprintf(struct ins *ins __maybe_unused, char *bf, size_t size,
> >  	return scnprintf(bf, size, "%-6.6s", "nop");
> >  }
> >  
> > -static struct ins_ops nop_ops = {
> > +static struct ins_ops nop_ops __maybe_unused = {
> >  	.scnprintf = nop__scnprintf,
> >  };
> >  
> > -static struct ins instructions[] = {
> > -	{ .name = "add",   .ops  = &mov_ops, },
> > -	{ .name = "addl",  .ops  = &mov_ops, },
> > -	{ .name = "addq",  .ops  = &mov_ops, },
> > -	{ .name = "addw",  .ops  = &mov_ops, },
> > -	{ .name = "and",   .ops  = &mov_ops, },
> > -#ifdef __arm__
> > -	{ .name = "b",     .ops  = &jump_ops, }, // might also be a call
> > -	{ .name = "bcc",   .ops  = &jump_ops, },
> > -	{ .name = "bcs",   .ops  = &jump_ops, },
> > -	{ .name = "beq",   .ops  = &jump_ops, },
> > -	{ .name = "bge",   .ops  = &jump_ops, },
> > -	{ .name = "bgt",   .ops  = &jump_ops, },
> > -	{ .name = "bhi",   .ops  = &jump_ops, },
> > -	{ .name = "bl",    .ops  = &call_ops, },
> > -	{ .name = "bls",   .ops  = &jump_ops, },
> > -	{ .name = "blt",   .ops  = &jump_ops, },
> > -	{ .name = "blx",   .ops  = &call_ops, },
> > -	{ .name = "bne",   .ops  = &jump_ops, },
> > -#endif
> > -	{ .name = "bts",   .ops  = &mov_ops, },
> > -	{ .name = "call",  .ops  = &call_ops, },
> > -	{ .name = "callq", .ops  = &call_ops, },
> > -	{ .name = "cmp",   .ops  = &mov_ops, },
> > -	{ .name = "cmpb",  .ops  = &mov_ops, },
> > -	{ .name = "cmpl",  .ops  = &mov_ops, },
> > -	{ .name = "cmpq",  .ops  = &mov_ops, },
> > -	{ .name = "cmpw",  .ops  = &mov_ops, },
> > -	{ .name = "cmpxch", .ops  = &mov_ops, },
> > -	{ .name = "dec",   .ops  = &dec_ops, },
> > -	{ .name = "decl",  .ops  = &dec_ops, },
> > -	{ .name = "imul",  .ops  = &mov_ops, },
> > -	{ .name = "inc",   .ops  = &dec_ops, },
> > -	{ .name = "incl",  .ops  = &dec_ops, },
> > -	{ .name = "ja",	   .ops  = &jump_ops, },
> > -	{ .name = "jae",   .ops  = &jump_ops, },
> > -	{ .name = "jb",	   .ops  = &jump_ops, },
> > -	{ .name = "jbe",   .ops  = &jump_ops, },
> > -	{ .name = "jc",	   .ops  = &jump_ops, },
> > -	{ .name = "jcxz",  .ops  = &jump_ops, },
> > -	{ .name = "je",	   .ops  = &jump_ops, },
> > -	{ .name = "jecxz", .ops  = &jump_ops, },
> > -	{ .name = "jg",	   .ops  = &jump_ops, },
> > -	{ .name = "jge",   .ops  = &jump_ops, },
> > -	{ .name = "jl",    .ops  = &jump_ops, },
> > -	{ .name = "jle",   .ops  = &jump_ops, },
> > -	{ .name = "jmp",   .ops  = &jump_ops, },
> > -	{ .name = "jmpq",  .ops  = &jump_ops, },
> > -	{ .name = "jna",   .ops  = &jump_ops, },
> > -	{ .name = "jnae",  .ops  = &jump_ops, },
> > -	{ .name = "jnb",   .ops  = &jump_ops, },
> > -	{ .name = "jnbe",  .ops  = &jump_ops, },
> > -	{ .name = "jnc",   .ops  = &jump_ops, },
> > -	{ .name = "jne",   .ops  = &jump_ops, },
> > -	{ .name = "jng",   .ops  = &jump_ops, },
> > -	{ .name = "jnge",  .ops  = &jump_ops, },
> > -	{ .name = "jnl",   .ops  = &jump_ops, },
> > -	{ .name = "jnle",  .ops  = &jump_ops, },
> > -	{ .name = "jno",   .ops  = &jump_ops, },
> > -	{ .name = "jnp",   .ops  = &jump_ops, },
> > -	{ .name = "jns",   .ops  = &jump_ops, },
> > -	{ .name = "jnz",   .ops  = &jump_ops, },
> > -	{ .name = "jo",	   .ops  = &jump_ops, },
> > -	{ .name = "jp",	   .ops  = &jump_ops, },
> > -	{ .name = "jpe",   .ops  = &jump_ops, },
> > -	{ .name = "jpo",   .ops  = &jump_ops, },
> > -	{ .name = "jrcxz", .ops  = &jump_ops, },
> > -	{ .name = "js",	   .ops  = &jump_ops, },
> > -	{ .name = "jz",	   .ops  = &jump_ops, },
> > -	{ .name = "lea",   .ops  = &mov_ops, },
> > -	{ .name = "lock",  .ops  = &lock_ops, },
> > -	{ .name = "mov",   .ops  = &mov_ops, },
> > -	{ .name = "movb",  .ops  = &mov_ops, },
> > -	{ .name = "movdqa",.ops  = &mov_ops, },
> > -	{ .name = "movl",  .ops  = &mov_ops, },
> > -	{ .name = "movq",  .ops  = &mov_ops, },
> > -	{ .name = "movslq", .ops  = &mov_ops, },
> > -	{ .name = "movzbl", .ops  = &mov_ops, },
> > -	{ .name = "movzwl", .ops  = &mov_ops, },
> > -	{ .name = "nop",   .ops  = &nop_ops, },
> > -	{ .name = "nopl",  .ops  = &nop_ops, },
> > -	{ .name = "nopw",  .ops  = &nop_ops, },
> > -	{ .name = "or",    .ops  = &mov_ops, },
> > -	{ .name = "orl",   .ops  = &mov_ops, },
> > -	{ .name = "test",  .ops  = &mov_ops, },
> > -	{ .name = "testb", .ops  = &mov_ops, },
> > -	{ .name = "testl", .ops  = &mov_ops, },
> > -	{ .name = "xadd",  .ops  = &mov_ops, },
> > -	{ .name = "xbeginl", .ops  = &jump_ops, },
> > -	{ .name = "xbeginq", .ops  = &jump_ops, },
> > -};
> > +static struct ins instructions[] = ARCH_INSTRUCTIONS;
> >  
> >  static int ins__key_cmp(const void *name, const void *insp)
> >  {
> > diff --git a/tools/perf/util/annotate_ins.h b/tools/perf/util/annotate_ins.h
> > new file mode 100644
> > index 0000000..2ec9a05
> > --- /dev/null
> > +++ b/tools/perf/util/annotate_ins.h
> > @@ -0,0 +1,10 @@
> > +#ifndef __ANNOTATE_INS_H
> > +#define __ANNOTATE_INS_H
> > +
> > +#ifdef HAVE_ANNOTATE_INS_SUPPORT
> > +#include <annotate_ins.h>
> > +#else
> > +#define ARCH_INSTRUCTIONS { }
> > +#endif
> > +
> > +#endif /* __ANNOTATE_INS_H */
> > -- 
> > 2.1.4
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:perf/urgent] perf annotate: Fix identification of ARM blt and bls instructions
  2016-05-19 16:59 ` [PATCH 1/7] perf annotate: Fix identification of ARM blt and bls instructions Chris Ryder
@ 2016-05-20 17:45   ` tip-bot for Chris Ryder
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Chris Ryder @ 2016-05-20 17:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: will.deacon, chris.ryder, mingo, peterz, alexander.shishkin, hpa,
	pawel.moll, acme, tglx, mark.rutland, linux-kernel

Commit-ID:  58c0400176b2cd35da43f3115fa94ca937483aca
Gitweb:     http://git.kernel.org/tip/58c0400176b2cd35da43f3115fa94ca937483aca
Author:     Chris Ryder <chris.ryder@arm.com>
AuthorDate: Thu, 19 May 2016 17:59:45 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 20 May 2016 11:43:57 -0300

perf annotate: Fix identification of ARM blt and bls instructions

The ARM blt and bls instructions are not correctly identified when
parsing assembly because the list of recognised instructions must be
sorted by name. Swap the ordering of blt and bls.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/560e196b7c79b7ff853caae13d8719a31479cb1a.1463676839.git.chris.ryder@arm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index b811924..3d9f2ca 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -372,8 +372,8 @@ static struct ins instructions[] = {
 	{ .name = "bgt",   .ops  = &jump_ops, },
 	{ .name = "bhi",   .ops  = &jump_ops, },
 	{ .name = "bl",    .ops  = &call_ops, },
-	{ .name = "blt",   .ops  = &jump_ops, },
 	{ .name = "bls",   .ops  = &jump_ops, },
+	{ .name = "blt",   .ops  = &jump_ops, },
 	{ .name = "blx",   .ops  = &call_ops, },
 	{ .name = "bne",   .ops  = &jump_ops, },
 #endif

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:perf/urgent] perf annotate: Sort list of recognised instructions
  2016-05-19 16:59 ` [PATCH 2/7] perf annotate: Sort list of recognised instructions Chris Ryder
@ 2016-05-20 17:45   ` tip-bot for Chris Ryder
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Chris Ryder @ 2016-05-20 17:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, tglx, linux-kernel, chris.ryder, mark.rutland, mingo,
	hpa, acme, alexander.shishkin, will.deacon, pawel.moll

Commit-ID:  7e4c1498130d7b6c26e6669839af4c7e321c9fec
Gitweb:     http://git.kernel.org/tip/7e4c1498130d7b6c26e6669839af4c7e321c9fec
Author:     Chris Ryder <chris.ryder@arm.com>
AuthorDate: Thu, 19 May 2016 17:59:46 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 20 May 2016 11:43:57 -0300

perf annotate: Sort list of recognised instructions

Currently the list of instructions recognised by perf annotate has to be
explicitly written in sorted order. This makes it easy to make mistakes
when adding new instructions. Sort the list of instructions on first
access.

Signed-off-by: Chris Ryder <chris.ryder@arm.com>
Acked-by: Pawel Moll <pawel.moll@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/4268febaf32f47f322c166fb2fe98cfec7041e11.1463676839.git.chris.ryder@arm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 3d9f2ca..7e5a1e8 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -354,9 +354,6 @@ static struct ins_ops nop_ops = {
 	.scnprintf = nop__scnprintf,
 };
 
-/*
- * Must be sorted by name!
- */
 static struct ins instructions[] = {
 	{ .name = "add",   .ops  = &mov_ops, },
 	{ .name = "addl",  .ops  = &mov_ops, },
@@ -449,18 +446,39 @@ static struct ins instructions[] = {
 	{ .name = "xbeginq", .ops  = &jump_ops, },
 };
 
-static int ins__cmp(const void *name, const void *insp)
+static int ins__key_cmp(const void *name, const void *insp)
 {
 	const struct ins *ins = insp;
 
 	return strcmp(name, ins->name);
 }
 
+static int ins__cmp(const void *a, const void *b)
+{
+	const struct ins *ia = a;
+	const struct ins *ib = b;
+
+	return strcmp(ia->name, ib->name);
+}
+
+static void ins__sort(void)
+{
+	const int nmemb = ARRAY_SIZE(instructions);
+
+	qsort(instructions, nmemb, sizeof(struct ins), ins__cmp);
+}
+
 static struct ins *ins__find(const char *name)
 {
 	const int nmemb = ARRAY_SIZE(instructions);
+	static bool sorted;
+
+	if (!sorted) {
+		ins__sort();
+		sorted = true;
+	}
 
-	return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__cmp);
+	return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__key_cmp);
 }
 
 int symbol__annotate_init(struct map *map __maybe_unused, struct symbol *sym)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-05-20 17:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-19 16:59 [PATCH 0/7] perf annotate: Support for AArch64 Chris Ryder
2016-05-19 16:59 ` [PATCH 1/7] perf annotate: Fix identification of ARM blt and bls instructions Chris Ryder
2016-05-20 17:45   ` [tip:perf/urgent] " tip-bot for Chris Ryder
2016-05-19 16:59 ` [PATCH 2/7] perf annotate: Sort list of recognised instructions Chris Ryder
2016-05-20 17:45   ` [tip:perf/urgent] " tip-bot for Chris Ryder
2016-05-19 16:59 ` [PATCH 3/7] pref annotate: Separate architecture specific annotation support Chris Ryder
2016-05-19 19:45   ` Arnaldo Carvalho de Melo
2016-05-20  9:44     ` Chris Ryder
2016-05-19 16:59 ` [PATCH 4/7] perf annotate: Separate out architecture specific parsing Chris Ryder
2016-05-19 16:59 ` [PATCH 5/7] perf annotate: Architecture neutral handling of return instruction Chris Ryder
2016-05-19 16:59 ` [PATCH 6/7] perf annotate: Make action message be architecture specific Chris Ryder
2016-05-19 16:59 ` [PATCH 7/7] perf annotate: AArch64 support Chris Ryder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).