perf annotate: Fix fused instr logic for assembly functions

Some x86 microarchitectures fuse a subset of cmp/test/ALU instructions
with branch instructions, and thus perf annotate highlight such valid
pairs as fused.

When annotated with source, perf uses struct disasm_line to contain
either source or instruction line from objdump output. Usually, a C
statement generates multiple instructions which include such
cmp/test/ALU + branch instruction pairs. But in case of assembly
function, each individual assembly source line generate one
instruction.

The 'perf annotate' instruction fusion logic assumes the previous
disasm_line as the previous instruction line, which is wrong because,
for assembly function, previous disasm_line contains source line.  And
thus perf fails to highlight valid fused instruction pairs for assembly
functions.

Fix it by searching backward until we find an instruction line and
consider that disasm_line as fused with current branch instruction.

Before:
         │    cmpq    %rcx, RIP+8(%rsp)
    0.00 │      cmp    %rcx,0x88(%rsp)
         │    je      .Lerror_bad_iret      <--- Source line
    0.14 │   ┌──je     b4                   <--- Instruction line
         │   │movl    %ecx, %eax

After:
         │    cmpq    %rcx, RIP+8(%rsp)
    0.00 │   ┌──cmp    %rcx,0x88(%rsp)
         │   │je      .Lerror_bad_iret
    0.14 │   ├──je     b4
         │   │movl    %ecx, %eax

Reviewed-by: Jin Yao <yao.jin@linux.intel.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kim Phillips <kim.phillips@amd.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https //lore.kernel.org/r/20210911043854.8373-1-ravi.bangoria@amd.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 781afe42e90e0455be687c577a84665d9ee36cbc..fa5bd5c20e96b011320d0b0dfdf99c095c98c4d2 100644 (file)
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -757,25 +757,40 @@ void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
 }
 
 void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
-                           unsigned int row, bool arrow_down)
+                           unsigned int row, int diff, bool arrow_down)
 {
-       unsigned int end_row;
+       int end_row;
 
-       if (row >= browser->top_idx)
-               end_row = row - browser->top_idx;
-       else
+       if (diff <= 0)
                return;
 
        SLsmg_set_char_set(1);
 
        if (arrow_down) {
+               if (row + diff <= browser->top_idx)
+                       return;
+
+               end_row = row + diff - browser->top_idx;
                ui_browser__gotorc(browser, end_row, column - 1);
-               SLsmg_write_char(SLSMG_ULCORN_CHAR);
-               ui_browser__gotorc(browser, end_row, column);
-               SLsmg_draw_hline(2);
-               ui_browser__gotorc(browser, end_row + 1, column - 1);
                SLsmg_write_char(SLSMG_LTEE_CHAR);
+
+               while (--end_row >= 0 && end_row > (int)(row - browser->top_idx)) {
+                       ui_browser__gotorc(browser, end_row, column - 1);
+                       SLsmg_draw_vline(1);
+               }
+
+               end_row = (int)(row - browser->top_idx);
+               if (end_row >= 0) {
+                       ui_browser__gotorc(browser, end_row, column - 1);
+                       SLsmg_write_char(SLSMG_ULCORN_CHAR);
+                       ui_browser__gotorc(browser, end_row, column);
+                       SLsmg_draw_hline(2);
+               }
        } else {
+               if (row < browser->top_idx)
+                       return;
+
+               end_row = row - browser->top_idx;
                ui_browser__gotorc(browser, end_row, column - 1);
                SLsmg_write_char(SLSMG_LTEE_CHAR);
                ui_browser__gotorc(browser, end_row, column);

diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 3678eb88f119cf5f09db2708c1ff74254fe85962..510ce45540501972942c0c3a34f6429ca97de690 100644 (file)
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -51,7 +51,7 @@ void ui_browser__write_graph(struct ui_browser *browser, int graph);
 void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
                              u64 start, u64 end);
 void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
-                           unsigned int row, bool arrow_down);
+                           unsigned int row, int diff, bool arrow_down);
 void __ui_browser__show_title(struct ui_browser *browser, const char *title);
 void ui_browser__show_title(struct ui_browser *browser, const char *title);
 int ui_browser__show(struct ui_browser *browser, const char *title,

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index ef4da4295bf7f774635b72b0f8cb602246446f84..e81c2493efdf9eb1884a7471821876e370387ecc 100644 (file)
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -125,13 +125,20 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
                ab->selection = al;
 }
 
-static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
+static int is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
 {
        struct disasm_line *pos = list_prev_entry(cursor, al.node);
        const char *name;
+       int diff = 1;
+
+       while (pos && pos->al.offset == -1) {
+               pos = list_prev_entry(pos, al.node);
+               if (!ab->opts->hide_src_code)
+                       diff++;
+       }
 
        if (!pos)
-               return false;
+               return 0;
 
        if (ins__is_lock(&pos->ins))
                name = pos->ops.locked.ins.name;
@@ -139,9 +146,11 @@ static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
                name = pos->ins.name;
 
        if (!name || !cursor->ins.name)
-               return false;
+               return 0;
 
-       return ins__is_fused(ab->arch, name, cursor->ins.name);
+       if (ins__is_fused(ab->arch, name, cursor->ins.name))
+               return diff;
+       return 0;
 }
 
 static void annotate_browser__draw_current_jump(struct ui_browser *browser)
@@ -155,6 +164,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
        struct annotation *notes = symbol__annotation(sym);
        u8 pcnt_width = annotation__pcnt_width(notes);
        int width;
+       int diff = 0;
 
        /* PLT symbols contain external offsets */
        if (strstr(sym->name, "@plt"))
@@ -205,11 +215,11 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
                                 pcnt_width + 2 + notes->widths.addr + width,
                                 from, to);
 
-       if (is_fused(ab, cursor)) {
+       diff = is_fused(ab, cursor);
+       if (diff > 0) {
                ui_browser__mark_fused(browser,
                                       pcnt_width + 3 + notes->widths.addr + width,
-                                      from - 1,
-                                      to > from);
+                                      from - diff, diff, to > from);
        }
 }