perf annotate: Remove duplicate 'name' field from disasm_line

The disasm_line::name field is always equal to ins::name, being used
just to locate the instruction's ins_ops from the per-arch instructions
table.

Eliminate this duplication, nuking that field and instead make
ins__find() return an ins_ops, store it in disasm_line::ins.ops, and
keep just in disasm_line::ins.name what was in disasm_line::name, this
way we end up not keeping a reference to entries in the per-arch
instructions table.

This in turn will help supporting multiple ways to manage the per-arch
instructions table, allowing resorting that array, for instance, when
the entries will move after references to its addresses were made. The
same problem is avoided when one grows the array with realloc.

So architectures simply keeping a constant array will work as well as
architectures building the table using regular expressions or other
logic that involves resorting the table.

Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Chris Riyder <chris.ryder@arm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kim Phillips <kim.phillips@arm.com>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Taeung Song <treeze.taeung@gmail.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-vr899azvabnw9gtuepuqfd9t@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index e6e9f7d80dbdcb39ea5f17570ef5f39b6cc5d8b1..cee0eee31ce6de28c2559a0f17b860a2a083cad3 100644 (file)
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -213,17 +213,17 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
                ui_browser__write_nstring(browser, bf, printed);
                if (change_color)
                        ui_browser__set_color(browser, color);
-               if (dl->ins && dl->ins->ops->scnprintf) {
-                       if (ins__is_jump(dl->ins)) {
+               if (dl->ins.ops && dl->ins.ops->scnprintf) {
+                       if (ins__is_jump(&dl->ins)) {
                                bool fwd = dl->ops.target.offset > (u64)dl->offset;
 
                                ui_browser__write_graph(browser, fwd ? SLSMG_DARROW_CHAR :
                                                                    SLSMG_UARROW_CHAR);
                                SLsmg_write_char(' ');
-                       } else if (ins__is_call(dl->ins)) {
+                       } else if (ins__is_call(&dl->ins)) {
                                ui_browser__write_graph(browser, SLSMG_RARROW_CHAR);
                                SLsmg_write_char(' ');
-                       } else if (ins__is_ret(dl->ins)) {
+                       } else if (ins__is_ret(&dl->ins)) {
                                ui_browser__write_graph(browser, SLSMG_LARROW_CHAR);
                                SLsmg_write_char(' ');
                        } else {
@@ -243,7 +243,7 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 
 static bool disasm_line__is_valid_jump(struct disasm_line *dl, struct symbol *sym)
 {
-       if (!dl || !dl->ins || !ins__is_jump(dl->ins)
+       if (!dl || !dl->ins.ops || !ins__is_jump(&dl->ins)
            || !disasm_line__has_offset(dl)
            || dl->ops.target.offset >= symbol__size(sym))
                return false;
@@ -492,7 +492,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
        };
        char title[SYM_TITLE_MAX_SIZE];
 
-       if (!ins__is_call(dl->ins))
+       if (!ins__is_call(&dl->ins))
                return false;
 
        if (map_groups__find_ams(&target) ||
@@ -545,7 +545,7 @@ static bool annotate_browser__jump(struct annotate_browser *browser)
        struct disasm_line *dl = browser->selection;
        s64 idx;
 
-       if (!ins__is_jump(dl->ins))
+       if (!ins__is_jump(&dl->ins))
                return false;
 
        dl = annotate_browser__find_offset(browser, dl->ops.target.offset, &idx);
@@ -841,9 +841,9 @@ show_help:
                                ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org");
                        else if (browser->selection->offset == -1)
                                ui_helpline__puts("Actions are only available for assembly lines.");
-                       else if (!browser->selection->ins)
+                       else if (!browser->selection->ins.ops)
                                goto show_sup_ins;
-                       else if (ins__is_ret(browser->selection->ins))
+                       else if (ins__is_ret(&browser->selection->ins))
                                goto out;
                        else if (!(annotate_browser__jump(browser) ||
                                     annotate_browser__callq(browser, evsel, hbt))) {

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 095d90a9077f05b4ad04c0114f44d0d67b4893e3..b48a39be071b2af7813e326a892ddf4ce007e814 100644 (file)
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -28,8 +28,8 @@ const char    *disassembler_style;
 const char     *objdump_path;
 static regex_t  file_lineno;
 
-static struct ins *ins__find(struct arch *arch, const char *name);
-static int disasm_line__parse(char *line, char **namep, char **rawp);
+static struct ins_ops *ins__find(struct arch *arch, const char *name);
+static int disasm_line__parse(char *line, const char **namep, char **rawp);
 
 struct arch {
        const char      *name;
@@ -218,26 +218,20 @@ static int comment__symbol(char *raw, char *comment, u64 *addrp, char **namep)
 
 static int lock__parse(struct arch *arch, struct ins_operands *ops, struct map *map)
 {
-       char *name;
-
        ops->locked.ops = zalloc(sizeof(*ops->locked.ops));
        if (ops->locked.ops == NULL)
                return 0;
 
-       if (disasm_line__parse(ops->raw, &name, &ops->locked.ops->raw) < 0)
+       if (disasm_line__parse(ops->raw, &ops->locked.ins.name, &ops->locked.ops->raw) < 0)
                goto out_free_ops;
 
-       ops->locked.ins = ins__find(arch, name);
-       free(name);
+       ops->locked.ins.ops = ins__find(arch, ops->locked.ins.name);
 
-       if (ops->locked.ins == NULL)
+       if (ops->locked.ins.ops == NULL)
                goto out_free_ops;
 
-       if (!ops->locked.ins->ops)
-               return 0;
-
-       if (ops->locked.ins->ops->parse &&
-           ops->locked.ins->ops->parse(arch, ops->locked.ops, map) < 0)
+       if (ops->locked.ins.ops->parse &&
+           ops->locked.ins.ops->parse(arch, ops->locked.ops, map) < 0)
                goto out_free_ops;
 
        return 0;
@@ -252,19 +246,19 @@ static int lock__scnprintf(struct ins *ins, char *bf, size_t size,
 {
        int printed;
 
-       if (ops->locked.ins == NULL)
+       if (ops->locked.ins.ops == NULL)
                return ins__raw_scnprintf(ins, bf, size, ops);
 
        printed = scnprintf(bf, size, "%-6.6s ", ins->name);
-       return printed + ins__scnprintf(ops->locked.ins, bf + printed,
+       return printed + ins__scnprintf(&ops->locked.ins, bf + printed,
                                        size - printed, ops->locked.ops);
 }
 
 static void lock__delete(struct ins_operands *ops)
 {
-       struct ins *ins = ops->locked.ins;
+       struct ins *ins = &ops->locked.ins;
 
-       if (ins && ins->ops->free)
+       if (ins->ops && ins->ops->free)
                ins->ops->free(ops->locked.ops);
        else
                ins__delete(ops->locked.ops);
@@ -425,8 +419,9 @@ static void ins__sort(struct arch *arch)
        qsort(arch->instructions, nmemb, sizeof(struct ins), ins__cmp);
 }
 
-static struct ins *ins__find(struct arch *arch, const char *name)
+static struct ins_ops *ins__find(struct arch *arch, const char *name)
 {
+       struct ins *ins;
        const int nmemb = arch->nr_instructions;
 
        if (!arch->sorted_instructions) {
@@ -434,7 +429,8 @@ static struct ins *ins__find(struct arch *arch, const char *name)
                arch->sorted_instructions = true;
        }
 
-       return bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp);
+       ins = bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp);
+       return ins ? ins->ops : NULL;
 }
 
 static int arch__key_cmp(const void *name, const void *archp)
@@ -691,19 +687,16 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
 
 static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, struct map *map)
 {
-       dl->ins = ins__find(arch, dl->name);
-
-       if (dl->ins == NULL)
-               return;
+       dl->ins.ops = ins__find(arch, dl->ins.name);
 
-       if (!dl->ins->ops)
+       if (!dl->ins.ops)
                return;
 
-       if (dl->ins->ops->parse && dl->ins->ops->parse(arch, &dl->ops, map) < 0)
-               dl->ins = NULL;
+       if (dl->ins.ops->parse && dl->ins.ops->parse(arch, &dl->ops, map) < 0)
+               dl->ins.ops = NULL;
 }
 
-static int disasm_line__parse(char *line, char **namep, char **rawp)
+static int disasm_line__parse(char *line, const char **namep, char **rawp)
 {
        char *name = line, tmp;
 
@@ -736,7 +729,8 @@ static int disasm_line__parse(char *line, char **namep, char **rawp)
        return 0;
 
 out_free_name:
-       zfree(namep);
+       free((void *)namep);
+       *namep = NULL;
        return -1;
 }
 
@@ -755,7 +749,7 @@ static struct disasm_line *disasm_line__new(s64 offset, char *line,
                        goto out_delete;
 
                if (offset != -1) {
-                       if (disasm_line__parse(dl->line, &dl->name, &dl->ops.raw) < 0)
+                       if (disasm_line__parse(dl->line, &dl->ins.name, &dl->ops.raw) < 0)
                                goto out_free_line;
 
                        disasm_line__init_ins(dl, arch, map);
@@ -774,20 +768,21 @@ out_delete:
 void disasm_line__free(struct disasm_line *dl)
 {
        zfree(&dl->line);
-       zfree(&dl->name);
-       if (dl->ins && dl->ins->ops->free)
-               dl->ins->ops->free(&dl->ops);
+       if (dl->ins.ops && dl->ins.ops->free)
+               dl->ins.ops->free(&dl->ops);
        else
                ins__delete(&dl->ops);
+       free((void *)dl->ins.name);
+       dl->ins.name = NULL;
        free(dl);
 }
 
 int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw)
 {
-       if (raw || !dl->ins)
-               return scnprintf(bf, size, "%-6.6s %s", dl->name, dl->ops.raw);
+       if (raw || !dl->ins.ops)
+               return scnprintf(bf, size, "%-6.6s %s", dl->ins.name, dl->ops.raw);
 
-       return ins__scnprintf(dl->ins, bf, size, &dl->ops);
+       return ins__scnprintf(&dl->ins, bf, size, &dl->ops);
 }
 
 static void disasm__add(struct list_head *head, struct disasm_line *line)
@@ -1143,7 +1138,7 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
                                        map__rip_2objdump(map, sym->start);
 
        /* kcore has no symbols, so add the call target name */
-       if (dl->ins && ins__is_call(dl->ins) && !dl->ops.target.name) {
+       if (dl->ins.ops && ins__is_call(&dl->ins) && !dl->ops.target.name) {
                struct addr_map_symbol target = {
                        .map = map,
                        .addr = dl->ops.target.addr,
@@ -1173,8 +1168,8 @@ static void delete_last_nop(struct symbol *sym)
        while (!list_empty(list)) {
                dl = list_entry(list->prev, struct disasm_line, node);
 
-               if (dl->ins && dl->ins->ops) {
-                       if (dl->ins->ops != &nop_ops)
+               if (dl->ins.ops) {
+                       if (dl->ins.ops != &nop_ops)
                                return;
                } else {
                        if (!strstr(dl->line, " nop ") &&
@@ -1767,7 +1762,7 @@ static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
        if (dl->offset == -1)
                return fprintf(fp, "%s\n", dl->line);
 
-       printed = fprintf(fp, "%#" PRIx64 " %s", dl->offset, dl->name);
+       printed = fprintf(fp, "%#" PRIx64 " %s", dl->offset, dl->ins.name);
 
        if (dl->ops.raw[0] != '\0') {
                printed += fprintf(fp, "%.*s %s\n", 6 - (int)printed, " ",

diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 8e490b5c91bc5c237afb9f7d46b008f0e0f47df3..87e4cadc5d2758ce4603428560c9f8548e4fc851 100644 (file)
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -11,7 +11,12 @@
 #include <linux/rbtree.h>
 #include <pthread.h>
 
-struct ins;
+struct ins_ops;
+
+struct ins {
+       const char     *name;
+       struct ins_ops *ops;
+};
 
 struct ins_operands {
        char    *raw;
@@ -28,7 +33,7 @@ struct ins_operands {
                        u64     addr;
                } source;
                struct {
-                       struct ins *ins;
+                       struct ins          ins;
                        struct ins_operands *ops;
                } locked;
        };
@@ -43,11 +48,6 @@ struct ins_ops {
                         struct ins_operands *ops);
 };
 
-struct ins {
-       const char     *name;
-       struct ins_ops *ops;
-};
-
 bool ins__is_jump(const struct ins *ins);
 bool ins__is_call(const struct ins *ins);
 bool ins__is_ret(const struct ins *ins);
@@ -59,8 +59,7 @@ struct disasm_line {
        struct list_head    node;
        s64                 offset;
        char                *line;
-       char                *name;
-       struct ins          *ins;
+       struct ins          ins;
        int                 line_nr;
        float               ipc;
        u64                 cycles;