Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1466377 > unrolled thread
| Started by | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| First post | 2016-08-19 15:10 +0200 |
| Last post | 2016-08-23 22:40 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v6 2/7] perf annotate: Add cross arch annotate support Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-08-19 15:10 +0200
Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support Kim Phillips <kim.phillips@arm.com> - 2016-08-23 01:10 +0200
Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support Namhyung Kim <namhyung@kernel.org> - 2016-08-23 04:20 +0200
Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support Kim Phillips <kim.phillips@arm.com> - 2016-08-23 22:40 +0200
| From | Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-08-19 15:10 +0200 |
| Subject | [PATCH v6 2/7] perf annotate: Add cross arch annotate support |
| Message-ID | <s7RIC-61N-29@gated-at.bofh.it> |
Change current data structures and function to enable cross arch
annotate.
Current perf implementation does not support cross arch annotate.
To make it truly cross arch, instruction table of all arch should
be present in perf binary. And use appropriate table based on arch
where perf.data was recorded.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
Changes in v6:
- Instead of adding only those instructions defined in #ifdef __arm__,
add all instructions from default table to arm table.
tools/perf/builtin-top.c | 2 +-
tools/perf/ui/browsers/annotate.c | 3 +-
tools/perf/ui/gtk/annotate.c | 2 +-
tools/perf/util/annotate.c | 187 ++++++++++++++++++++++++++++++--------
tools/perf/util/annotate.h | 5 +-
5 files changed, 157 insertions(+), 42 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index a3223aa..fdd4203 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -129,7 +129,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)
return err;
}
- err = symbol__disassemble(sym, map, 0);
+ err = symbol__disassemble(sym, map, 0, NULL);
if (err == 0) {
out_assign:
top->sym_filter_entry = he;
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 2e2d100..21c5e10 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -1050,7 +1050,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
(nr_pcnt - 1);
}
- err = symbol__disassemble(sym, map, sizeof_bdl);
+ err = symbol__disassemble(sym, map, sizeof_bdl,
+ perf_evsel__env_arch(evsel));
if (err) {
char msg[BUFSIZ];
symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index 42d3199..c127aba 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -167,7 +167,7 @@ static int symbol__gtk_annotate(struct symbol *sym, struct map *map,
if (map->dso->annotate_warned)
return -1;
- err = symbol__disassemble(sym, map, 0);
+ err = symbol__disassemble(sym, map, 0, perf_evsel__env_arch(evsel));
if (err) {
char msg[BUFSIZ];
symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 25a9259..14a8808 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -20,12 +20,14 @@
#include <regex.h>
#include <pthread.h>
#include <linux/bitops.h>
+#include <sys/utsname.h>
+#include "../arch/common.h"
const char *disassembler_style;
const char *objdump_path;
static regex_t file_lineno;
-static struct ins *ins__find(const char *name);
+static struct ins *ins__find(const char *name, const char *norm_arch);
static int disasm_line__parse(char *line, char **namep, char **rawp);
static void ins__delete(struct ins_operands *ops)
@@ -53,7 +55,7 @@ int ins__scnprintf(struct ins *ins, char *bf, size_t size,
return ins__raw_scnprintf(ins, bf, size, ops);
}
-static int call__parse(struct ins_operands *ops)
+static int call__parse(struct ins_operands *ops, const char *norm_arch)
{
char *endptr, *tok, *name;
@@ -65,10 +67,8 @@ static int call__parse(struct ins_operands *ops)
name++;
-#ifdef __arm__
- if (strchr(name, '+'))
+ if (!strcmp(norm_arch, NORM_ARM) && strchr(name, '+'))
return -1;
-#endif
tok = strchr(name, '>');
if (tok == NULL)
@@ -117,7 +117,8 @@ bool ins__is_call(const struct ins *ins)
return ins->ops == &call_ops;
}
-static int jump__parse(struct ins_operands *ops)
+static int jump__parse(struct ins_operands *ops,
+ const char *norm_arch __maybe_unused)
{
const char *s = strchr(ops->raw, '+');
@@ -172,7 +173,7 @@ static int comment__symbol(char *raw, char *comment, u64 *addrp, char **namep)
return 0;
}
-static int lock__parse(struct ins_operands *ops)
+static int lock__parse(struct ins_operands *ops, const char *norm_arch)
{
char *name;
@@ -183,7 +184,7 @@ static int lock__parse(struct ins_operands *ops)
if (disasm_line__parse(ops->raw, &name, &ops->locked.ops->raw) < 0)
goto out_free_ops;
- ops->locked.ins = ins__find(name);
+ ops->locked.ins = ins__find(name, norm_arch);
free(name);
if (ops->locked.ins == NULL)
@@ -193,7 +194,7 @@ static int lock__parse(struct ins_operands *ops)
return 0;
if (ops->locked.ins->ops->parse &&
- ops->locked.ins->ops->parse(ops->locked.ops) < 0)
+ ops->locked.ins->ops->parse(ops->locked.ops, norm_arch) < 0)
goto out_free_ops;
return 0;
@@ -236,7 +237,7 @@ static struct ins_ops lock_ops = {
.scnprintf = lock__scnprintf,
};
-static int mov__parse(struct ins_operands *ops)
+static int mov__parse(struct ins_operands *ops, const char *norm_arch)
{
char *s = strchr(ops->raw, ','), *target, *comment, prev;
@@ -251,11 +252,11 @@ static int mov__parse(struct ins_operands *ops)
return -1;
target = ++s;
-#ifdef __arm__
- comment = strchr(s, ';');
-#else
- comment = strchr(s, '#');
-#endif
+
+ if (!strcmp(norm_arch, NORM_ARM))
+ comment = strchr(s, ';');
+ else
+ comment = strchr(s, '#');
if (comment != NULL)
s = comment - 1;
@@ -303,7 +304,8 @@ static struct ins_ops mov_ops = {
.scnprintf = mov__scnprintf,
};
-static int dec__parse(struct ins_operands *ops)
+static int dec__parse(struct ins_operands *ops,
+ const char *norm_arch __maybe_unused)
{
char *target, *comment, *s, prev;
@@ -363,14 +365,92 @@ bool ins__is_ret(const struct ins *ins)
return ins->ops == &ret_ops;
}
-static struct ins instructions[] = {
+static struct ins instructions_x86[] = {
+ { .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, },
+ { .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, },
-#ifdef __arm__
- { .name = "b", .ops = &jump_ops, }, // might also be a call
+ { .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, },
@@ -382,7 +462,6 @@ static struct ins instructions[] = {
{ .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, },
@@ -471,24 +550,48 @@ static int ins__cmp(const void *a, const void *b)
return strcmp(ia->name, ib->name);
}
-static void ins__sort(void)
+static void ins__sort(struct ins *instructions, int nmemb)
{
- const int nmemb = ARRAY_SIZE(instructions);
-
qsort(instructions, nmemb, sizeof(struct ins), ins__cmp);
}
-static struct ins *ins__find(const char *name)
+static const char *annotate__norm_arch(char *arch)
+{
+ struct utsname uts;
+
+ if (!arch) { /* Assume we are annotating locally. */
+ if (uname(&uts) < 0)
+ return NULL;
+ arch = uts.machine;
+ }
+ return normalize_arch(arch);
+}
+
+static struct ins *ins__find(const char *name, const char *norm_arch)
{
- const int nmemb = ARRAY_SIZE(instructions);
static bool sorted;
+ struct ins *instructions;
+ int nmemb;
if (!sorted) {
- ins__sort();
+ ins__sort(instructions_x86, ARRAY_SIZE(instructions_x86));
+ ins__sort(instructions_arm, ARRAY_SIZE(instructions_arm));
sorted = true;
}
- return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__key_cmp);
+ if (!strcmp(norm_arch, NORM_X86)) {
+ instructions = instructions_x86;
+ nmemb = ARRAY_SIZE(instructions_x86);
+ } else if (!strcmp(norm_arch, NORM_ARM)) {
+ instructions = instructions_arm;
+ nmemb = ARRAY_SIZE(instructions_arm);
+ } else {
+ pr_err("perf annotate not supported by %s arch\n", norm_arch);
+ return NULL;
+ }
+
+ return bsearch(name, instructions, nmemb, sizeof(struct ins),
+ ins__key_cmp);
}
int symbol__annotate_init(struct map *map __maybe_unused, struct symbol *sym)
@@ -715,17 +818,24 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip);
}
-static void disasm_line__init_ins(struct disasm_line *dl)
+static void disasm_line__init_ins(struct disasm_line *dl, char *arch)
{
- dl->ins = ins__find(dl->name);
+ const char *norm_arch = annotate__norm_arch(arch);
+
+ if (!norm_arch) {
+ pr_err("Can not annotate. Could not determine architecture.");
+ return;
+ }
+ dl->ins = ins__find(dl->name, norm_arch);
if (dl->ins == NULL)
return;
if (!dl->ins->ops)
return;
- if (dl->ins->ops->parse && dl->ins->ops->parse(&dl->ops) < 0)
+ if (dl->ins->ops->parse &&
+ dl->ins->ops->parse(&dl->ops, norm_arch) < 0)
dl->ins = NULL;
}
@@ -767,7 +877,8 @@ out_free_name:
}
static struct disasm_line *disasm_line__new(s64 offset, char *line,
- size_t privsize, int line_nr)
+ size_t privsize, int line_nr,
+ char *arch)
{
struct disasm_line *dl = zalloc(sizeof(*dl) + privsize);
@@ -782,7 +893,7 @@ static struct disasm_line *disasm_line__new(s64 offset, char *line,
if (disasm_line__parse(dl->line, &dl->name, &dl->ops.raw) < 0)
goto out_free_line;
- disasm_line__init_ins(dl);
+ disasm_line__init_ins(dl, arch);
}
}
@@ -1005,7 +1116,7 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
*/
static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
FILE *file, size_t privsize,
- int *line_nr)
+ int *line_nr, char *arch)
{
struct annotation *notes = symbol__annotation(sym);
struct disasm_line *dl;
@@ -1066,7 +1177,8 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
parsed_line = tmp2 + 1;
}
- dl = disasm_line__new(offset, parsed_line, privsize, *line_nr);
+ dl = disasm_line__new(offset, parsed_line, privsize,
+ *line_nr, arch);
free(line);
(*line_nr)++;
@@ -1197,7 +1309,8 @@ fallback:
return 0;
}
-int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize)
+int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize,
+ char *arch)
{
struct dso *dso = map->dso;
char command[PATH_MAX * 2];
@@ -1313,7 +1426,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize)
nline = 0;
while (!feof(file)) {
if (symbol__parse_objdump_line(sym, map, file, privsize,
- &lineno) < 0)
+ &lineno, arch) < 0)
break;
nline++;
}
@@ -1710,7 +1823,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map,
struct rb_root source_line = RB_ROOT;
u64 len;
- if (symbol__disassemble(sym, map, 0) < 0)
+ if (symbol__disassemble(sym, map, 0, perf_evsel__env_arch(evsel)) < 0)
return -1;
len = symbol__size(sym);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index f67ccb0..5cfad4e 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -36,7 +36,7 @@ struct ins_operands {
struct ins_ops {
void (*free)(struct ins_operands *ops);
- int (*parse)(struct ins_operands *ops);
+ int (*parse)(struct ins_operands *ops, const char *norm_arch);
int (*scnprintf)(struct ins *ins, char *bf, size_t size,
struct ins_operands *ops);
};
@@ -155,7 +155,8 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 addr);
int symbol__alloc_hist(struct symbol *sym);
void symbol__annotate_zero_histograms(struct symbol *sym);
-int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize);
+int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize,
+ char *arch);
enum symbol_disassemble_errno {
SYMBOL_ANNOTATE_ERRNO__SUCCESS = 0,
--
2.5.5
[toc] | [next] | [standalone]
| From | Kim Phillips <kim.phillips@arm.com> |
|---|---|
| Date | 2016-08-23 01:10 +0200 |
| Message-ID | <s96vT-4jp-1@gated-at.bofh.it> |
| In reply to | #1466377 |
On Fri, 19 Aug 2016 18:29:33 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
> Changes in v6:
> - Instead of adding only those instructions defined in #ifdef __arm__,
> add all instructions from default table to arm table.
..
> +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, },
> -#ifdef __arm__
> - { .name = "b", .ops = &jump_ops, }, // might also be a call
> + { .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, },
> @@ -382,7 +462,6 @@ static struct ins instructions[] = {
> { .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, },
> @@ -471,24 +550,48 @@ static int ins__cmp(const void *a, const void *b)
> return strcmp(ia->name, ib->name);
> }
Thanks, I've gone through the list and removed all not-ARM
instructions, and added some missing ARM branch instructions:
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index b2c6cf3..9d686504 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -451,9 +451,6 @@ static struct ins instructions_x86[] = {
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, },
@@ -463,81 +460,20 @@ static struct ins instructions_arm[] = {
{ .name = "bgt", .ops = &jump_ops, },
{ .name = "bhi", .ops = &jump_ops, },
{ .name = "bl", .ops = &call_ops, },
+ { .name = "ble", .ops = &jump_ops, },
+ { .name = "bleq", .ops = &call_ops, },
+ { .name = "blne", .ops = &call_ops, },
{ .name = "bls", .ops = &jump_ops, },
{ .name = "blt", .ops = &jump_ops, },
{ .name = "blx", .ops = &call_ops, },
+ { .name = "blxne", .ops = &call_ops, },
+ { .name = "bmi", .ops = &jump_ops, },
{ .name = "bne", .ops = &jump_ops, },
- { .name = "bts", .ops = &mov_ops, },
- { .name = "call", .ops = &call_ops, },
- { .name = "callq", .ops = &call_ops, },
+ { .name = "bpl", .ops = &jump_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, },
- { .name = "retq", .ops = &ret_ops, },
};
struct instructions_powerpc {
Ideally ARM would - like this powerpc implementation - add a
ins__find_arm() to handle its condition codes, but, for now, is
it possible to merge this change into this series? I can post a
follow-up patch if not.
Thanks,
Kim
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-08-23 04:20 +0200 |
| Message-ID | <s99tL-6eX-1@gated-at.bofh.it> |
| In reply to | #1468136 |
Hello,
On Tue, Aug 23, 2016 at 8:01 AM, Kim Phillips <kim.phillips@arm.com> wrote:
> On Fri, 19 Aug 2016 18:29:33 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> Changes in v6:
>> - Instead of adding only those instructions defined in #ifdef __arm__,
>> add all instructions from default table to arm table.
> ..
>> +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, },
>> -#ifdef __arm__
>> - { .name = "b", .ops = &jump_ops, }, // might also be a call
>> + { .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, },
>> @@ -382,7 +462,6 @@ static struct ins instructions[] = {
>> { .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, },
>> @@ -471,24 +550,48 @@ static int ins__cmp(const void *a, const void *b)
>> return strcmp(ia->name, ib->name);
>> }
>
> Thanks, I've gone through the list and removed all not-ARM
> instructions, and added some missing ARM branch instructions:
Can we use regex patterns instead?
Thanks
Namhyung
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index b2c6cf3..9d686504 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -451,9 +451,6 @@ static struct ins instructions_x86[] = {
>
> 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, },
> @@ -463,81 +460,20 @@ static struct ins instructions_arm[] = {
> { .name = "bgt", .ops = &jump_ops, },
> { .name = "bhi", .ops = &jump_ops, },
> { .name = "bl", .ops = &call_ops, },
> + { .name = "ble", .ops = &jump_ops, },
> + { .name = "bleq", .ops = &call_ops, },
> + { .name = "blne", .ops = &call_ops, },
> { .name = "bls", .ops = &jump_ops, },
> { .name = "blt", .ops = &jump_ops, },
> { .name = "blx", .ops = &call_ops, },
> + { .name = "blxne", .ops = &call_ops, },
> + { .name = "bmi", .ops = &jump_ops, },
> { .name = "bne", .ops = &jump_ops, },
> - { .name = "bts", .ops = &mov_ops, },
> - { .name = "call", .ops = &call_ops, },
> - { .name = "callq", .ops = &call_ops, },
> + { .name = "bpl", .ops = &jump_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, },
> - { .name = "retq", .ops = &ret_ops, },
> };
>
> struct instructions_powerpc {
>
> Ideally ARM would - like this powerpc implementation - add a
> ins__find_arm() to handle its condition codes, but, for now, is
> it possible to merge this change into this series? I can post a
> follow-up patch if not.
>
> Thanks,
>
> Kim
[toc] | [prev] | [next] | [standalone]
| From | Kim Phillips <kim.phillips@arm.com> |
|---|---|
| Date | 2016-08-23 22:40 +0200 |
| Message-ID | <s9qEi-zs-45@gated-at.bofh.it> |
| In reply to | #1468230 |
On Tue, 23 Aug 2016 11:17:16 +0900
Namhyung Kim <namhyung@kernel.org> wrote:
> On Tue, Aug 23, 2016 at 8:01 AM, Kim Phillips <kim.phillips@arm.com> wrote:
> > On Fri, 19 Aug 2016 18:29:33 +0530
> > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
> >
> >> Changes in v6:
> >> - Instead of adding only those instructions defined in #ifdef __arm__,
> >> add all instructions from default table to arm table.
> > Thanks, I've gone through the list and removed all not-ARM
> > instructions, and added some missing ARM branch instructions:
>
> Can we use regex patterns instead?
Yes, that helps prevent mistakes updating instruction lists - how does
this look?:
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index b2c6cf3..52316f3 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -26,6 +26,7 @@
const char *disassembler_style;
const char *objdump_path;
static regex_t file_lineno;
+static regex_t arm_call_insn, arm_jump_insn;
static struct ins *ins__find(const char *name, const char *norm_arch);
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 = &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, },
- { .name = "retq", .ops = &ret_ops, },
-};
-
-struct instructions_powerpc {
+struct instructions_arch {
struct ins *ins;
struct list_head list;
};
@@ -560,41 +470,41 @@ static int ins__cmp(const void *a, const void *b)
return strcmp(ia->name, ib->name);
}
-static struct ins *list_add__ins_powerpc(struct instructions_powerpc *head,
+static struct ins *list_add__ins_arch(struct instructions_arch *head,
const char *name, struct ins_ops *ops)
{
- struct instructions_powerpc *ins_powerpc;
+ struct instructions_arch *ins_arch;
struct ins *ins;
ins = zalloc(sizeof(struct ins));
if (!ins)
return NULL;
- ins_powerpc = zalloc(sizeof(struct instructions_powerpc));
- if (!ins_powerpc)
+ ins_arch = zalloc(sizeof(struct instructions_arch));
+ if (!ins_arch)
goto out_free_ins;
ins->name = strdup(name);
if (!ins->name)
- goto out_free_ins_power;
+ goto out_free_ins_arch;
ins->ops = ops;
- ins_powerpc->ins = ins;
- list_add_tail(&(ins_powerpc->list), &(head->list));
+ ins_arch->ins = ins;
+ list_add_tail(&(ins_arch->list), &(head->list));
return ins;
-out_free_ins_power:
- zfree(&ins_powerpc);
+out_free_ins_arch:
+ zfree(&ins_arch);
out_free_ins:
zfree(&ins);
return NULL;
}
-static struct ins *list_search__ins_powerpc(struct instructions_powerpc *head,
+static struct ins *list_search__ins_arch(struct instructions_arch *head,
const char *name)
{
- struct instructions_powerpc *pos;
+ struct instructions_arch *pos;
list_for_each_entry(pos, &head->list, list) {
if (!strcmp(pos->ins->name, name))
@@ -608,7 +518,7 @@ static struct ins *ins__find_powerpc(const char *name)
int i;
struct ins *ins;
struct ins_ops *ops;
- static struct instructions_powerpc head;
+ static struct instructions_arch head;
static bool list_initialized;
/*
@@ -629,7 +539,7 @@ static struct ins *ins__find_powerpc(const char *name)
/*
* Return if we already have object of 'struct ins' for this instruction
*/
- ins = list_search__ins_powerpc(&head, name);
+ ins = list_search__ins_arch(&head, name);
if (ins)
return ins;
@@ -666,7 +576,44 @@ static struct ins *ins__find_powerpc(const char *name)
* Add instruction to list so next time no need to
* allocate memory for it.
*/
- return list_add__ins_powerpc(&head, name, ops);
+ return list_add__ins_arch(&head, name, ops);
+}
+
+static struct ins *ins__find_arm(const char *name)
+{
+ struct ins *ins;
+ struct ins_ops *ops = &mov_ops;
+ static struct instructions_arch head;
+ static bool list_initialized;
+ regmatch_t match[2];
+ int ret;
+
+ if (!list_initialized) {
+ INIT_LIST_HEAD(&head.list);
+ list_initialized = true;
+ }
+
+ /*
+ * Return if we already have object of 'struct ins' for this instruction
+ */
+ ins = list_search__ins_arch(&head, name);
+ if (ins)
+ return ins;
+
+ ret = regexec(&arm_call_insn, name, 2, match, 0);
+ if (!ret) {
+ ops = &call_ops;
+ } else {
+ ret = regexec(&arm_jump_insn, name, 2, match, 0);
+ if (!ret)
+ ops = &jump_ops;
+ }
+
+ /*
+ * Add instruction to list so next time no need to
+ * allocate memory for it.
+ */
+ return list_add__ins_arch(&head, name, ops);
}
static void ins__sort(struct ins *instructions, int nmemb)
@@ -686,15 +633,26 @@ static const char *annotate__norm_arch(char *arch)
return normalize_arch(arch);
}
+#define ARM_CONDS "(cc|cs|eq|ge|gt|hi|le|ls|lt|mi|ne|pl|vc|vs)"
+
static struct ins *ins__find(const char *name, const char *norm_arch)
{
static bool sorted;
struct ins *instructions;
- int nmemb;
+ int nmemb, ret;
if (!sorted) {
ins__sort(instructions_x86, ARRAY_SIZE(instructions_x86));
- ins__sort(instructions_arm, ARRAY_SIZE(instructions_arm));
+ if (!strcmp(norm_arch, NORM_ARM)) {
+ ret = regcomp(&arm_call_insn,
+ "^blx?" ARM_CONDS "?$", REG_EXTENDED);
+ ret |= regcomp(&arm_jump_insn,
+ "^bx?" ARM_CONDS "?$", REG_EXTENDED);
+ if (ret) {
+ pr_err("regcomp failed\n");
+ return NULL;
+ }
+ }
sorted = true;
}
@@ -702,8 +660,7 @@ static struct ins *ins__find(const char *name, const char *norm_arch)
instructions = instructions_x86;
nmemb = ARRAY_SIZE(instructions_x86);
} else if (!strcmp(norm_arch, NORM_ARM)) {
- instructions = instructions_arm;
- nmemb = ARRAY_SIZE(instructions_arm);
+ return ins__find_arm(name);
} else if (!strcmp(norm_arch, NORM_POWERPC)) {
return ins__find_powerpc(name);
} else {
Note that, for ARM, in order to match return instructions, 'lr' must be
found to be the operand of the branch instruction, which is not
contained in the 'name' variable passed to ins__find().
Kim
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web