Re: [PATCH 3/7] pref annotate: Separate architecture specific annotation support

From: Arnaldo Carvalho de Melo
Date: Thu May 19 2016 - 15:45:52 EST

Next message: Steve Muckle: "Re: [PATCH 5/5] cpufreq: schedutil: do not update rate limit ts when freq is unchanged"
Previous message: Paul E. McKenney: "Re: [PATCH 3/4] rcutorture: Make -soundhw a x86 specific option"
In reply to: Chris Ryder: "[PATCH 3/7] pref annotate: Separate architecture specific annotation support"
Next in thread: Chris Ryder: "Re: [PATCH 3/7] pref annotate: Separate architecture specific annotation support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxx>
> Acked-by: Pawel Moll <pawel.moll@xxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: linux-perf-users@xxxxxxxxxxxxxxx
> Cc: Will Deacon <will.deacon@xxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> ---
> 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

Next message: Steve Muckle: "Re: [PATCH 5/5] cpufreq: schedutil: do not update rate limit ts when freq is unchanged"
Previous message: Paul E. McKenney: "Re: [PATCH 3/4] rcutorture: Make -soundhw a x86 specific option"
In reply to: Chris Ryder: "[PATCH 3/7] pref annotate: Separate architecture specific annotation support"
Next in thread: Chris Ryder: "Re: [PATCH 3/7] pref annotate: Separate architecture specific annotation support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]