Re: [PATCH v7 6/6] perf annotate: cross arch annotate support fixes for ARM

2016-10-10 Thread Kim Phillips
On Mon, 10 Oct 2016 19:16:16 +0530
Ravi Bangoria  wrote:

> On Wednesday 05 October 2016 05:04 PM, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Sep 21, 2016 at 09:17:56PM +0530, Ravi Bangoria escreveu:
> >> From: Kim Phillips 
> >>
> >> For ARM we remove the list that contains non-arm insns, and
> >> instead add more maintainable branch instruction regex logic.
> > This one looks ok and actually is in the direction of having facilities
> > for all arches, should've come as infrastructure that then gets used by
> > ARM and powerpc.
> 
> This was authored by Kim and I didn't wanted to change that so I kept it
> at the end.

I don't mind if this gets merged higher up in the series, in fact, I
think it's preferred to adding the arm insn table and then removing it
in the same series.  Keeping my Author: isn't necessary either:
something like "Signed-off-by: Kim Phillips <...> [ARM bits]" will
suffice.

> I'm sending a cleanup patch that applies on top of this series. That patch
> moves most of arch specific stuff from util/annotate.c to
> util/annotate/.c. Please review it.
> 
> Please pull this series if you are ok with that patch. Otherwise I'll respin
> entire series.

I'll wait for the dust to settle here before submitting an ARM return
insn fix, and aarch64 support originally authored by Chris Ryder
(basically translate table in [1] into regex format).

Thanks,

Kim

[1] https://lkml.org/lkml/2016/5/19/461


Re: [PATCH v7 6/6] perf annotate: cross arch annotate support fixes for ARM

2016-10-10 Thread Ravi Bangoria


On Wednesday 05 October 2016 05:04 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 21, 2016 at 09:17:56PM +0530, Ravi Bangoria escreveu:
>> From: Kim Phillips 
>>
>> For ARM we remove the list that contains non-arm insns, and
>> instead add more maintainable branch instruction regex logic.
> This one looks ok and actually is in the direction of having facilities
> for all arches, should've come as infrastructure that then gets used by
> ARM and powerpc.

This was authored by Kim and I didn't wanted to change that so I kept it
at the end.

I'm sending a cleanup patch that applies on top of this series. That patch
moves most of arch specific stuff from util/annotate.c to
util/annotate/.c. Please review it.

Please pull this series if you are ok with that patch. Otherwise I'll respin
entire series.

Thanks
-Ravi



Re: [PATCH v7 6/6] perf annotate: cross arch annotate support fixes for ARM

2016-10-05 Thread Arnaldo Carvalho de Melo
Em Wed, Sep 21, 2016 at 09:17:56PM +0530, Ravi Bangoria escreveu:
> From: Kim Phillips 
> 
> For ARM we remove the list that contains non-arm insns, and
> instead add more maintainable branch instruction regex logic.

This one looks ok and actually is in the direction of having facilities
for all arches, should've come as infrastructure that then gets used by
ARM and powerpc.

- Arnaldo
 
> Signed-off-by: Kim Phillips 
> Signed-off-by: Ravi Bangoria 
> ---
> Changes in v7:
>   - Little bit change in initializing instruction list.
> 
>  tools/perf/util/annotate.c | 177 
> +
>  1 file changed, 65 insertions(+), 112 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index fc44dd1..83d5ac8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -28,6 +28,7 @@ const char  *disassembler_style;
>  const char   *objdump_path;
>  static regex_tfile_lineno;
>  static char  *norm_arch;
> +static regex_t   arm_call_insn, arm_jump_insn;
>  
>  static struct ins *ins__find(const char *name);
>  static int disasm_line__parse(char *line, char **namep, char **rawp);
> @@ -449,98 +450,7 @@ static struct ins instructions_x86[] = {
>   { .name = "retq",  .ops  = &ret_ops, },
>  };
>  
> -static struct ins instructions_arm[] = {
> - { .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 = "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 = "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 

[PATCH v7 6/6] perf annotate: cross arch annotate support fixes for ARM

2016-09-21 Thread Ravi Bangoria
From: Kim Phillips 

For ARM we remove the list that contains non-arm insns, and
instead add more maintainable branch instruction regex logic.

Signed-off-by: Kim Phillips 
Signed-off-by: Ravi Bangoria 
---
Changes in v7:
  - Little bit change in initializing instruction list.

 tools/perf/util/annotate.c | 177 +
 1 file changed, 65 insertions(+), 112 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index fc44dd1..83d5ac8 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -28,6 +28,7 @@ const char*disassembler_style;
 const char *objdump_path;
 static regex_t  file_lineno;
 static char*norm_arch;
+static regex_t arm_call_insn, arm_jump_insn;
 
 static struct ins *ins__find(const char *name);
 static int disasm_line__parse(char *line, char **namep, char **rawp);
@@ -449,98 +450,7 @@ static struct ins instructions_x86[] = {
{ .name = "retq",  .ops  = &ret_ops, },
 };
 
-static struct ins instructions_arm[] = {
-   { .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 = "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 = "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