Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Ravi Bangoria and committed by
Arnaldo Carvalho de Melo
7efbcc8c 93ff9f13

+42 -17
+24 -9
tools/perf/ui/browser.c
··· 757 757 } 758 758 759 759 void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column, 760 - unsigned int row, bool arrow_down) 760 + unsigned int row, int diff, bool arrow_down) 761 761 { 762 - unsigned int end_row; 762 + int end_row; 763 763 764 - if (row >= browser->top_idx) 765 - end_row = row - browser->top_idx; 766 - else 764 + if (diff <= 0) 767 765 return; 768 766 769 767 SLsmg_set_char_set(1); 770 768 771 769 if (arrow_down) { 770 + if (row + diff <= browser->top_idx) 771 + return; 772 + 773 + end_row = row + diff - browser->top_idx; 772 774 ui_browser__gotorc(browser, end_row, column - 1); 773 - SLsmg_write_char(SLSMG_ULCORN_CHAR); 774 - ui_browser__gotorc(browser, end_row, column); 775 - SLsmg_draw_hline(2); 776 - ui_browser__gotorc(browser, end_row + 1, column - 1); 777 775 SLsmg_write_char(SLSMG_LTEE_CHAR); 776 + 777 + while (--end_row >= 0 && end_row > (int)(row - browser->top_idx)) { 778 + ui_browser__gotorc(browser, end_row, column - 1); 779 + SLsmg_draw_vline(1); 780 + } 781 + 782 + end_row = (int)(row - browser->top_idx); 783 + if (end_row >= 0) { 784 + ui_browser__gotorc(browser, end_row, column - 1); 785 + SLsmg_write_char(SLSMG_ULCORN_CHAR); 786 + ui_browser__gotorc(browser, end_row, column); 787 + SLsmg_draw_hline(2); 788 + } 778 789 } else { 790 + if (row < browser->top_idx) 791 + return; 792 + 793 + end_row = row - browser->top_idx; 779 794 ui_browser__gotorc(browser, end_row, column - 1); 780 795 SLsmg_write_char(SLSMG_LTEE_CHAR); 781 796 ui_browser__gotorc(browser, end_row, column);
+1 -1
tools/perf/ui/browser.h
··· 51 51 void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column, 52 52 u64 start, u64 end); 53 53 void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column, 54 - unsigned int row, bool arrow_down); 54 + unsigned int row, int diff, bool arrow_down); 55 55 void __ui_browser__show_title(struct ui_browser *browser, const char *title); 56 56 void ui_browser__show_title(struct ui_browser *browser, const char *title); 57 57 int ui_browser__show(struct ui_browser *browser, const char *title,
+17 -7
tools/perf/ui/browsers/annotate.c
··· 125 125 ab->selection = al; 126 126 } 127 127 128 - static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor) 128 + static int is_fused(struct annotate_browser *ab, struct disasm_line *cursor) 129 129 { 130 130 struct disasm_line *pos = list_prev_entry(cursor, al.node); 131 131 const char *name; 132 + int diff = 1; 133 + 134 + while (pos && pos->al.offset == -1) { 135 + pos = list_prev_entry(pos, al.node); 136 + if (!ab->opts->hide_src_code) 137 + diff++; 138 + } 132 139 133 140 if (!pos) 134 - return false; 141 + return 0; 135 142 136 143 if (ins__is_lock(&pos->ins)) 137 144 name = pos->ops.locked.ins.name; ··· 146 139 name = pos->ins.name; 147 140 148 141 if (!name || !cursor->ins.name) 149 - return false; 142 + return 0; 150 143 151 - return ins__is_fused(ab->arch, name, cursor->ins.name); 144 + if (ins__is_fused(ab->arch, name, cursor->ins.name)) 145 + return diff; 146 + return 0; 152 147 } 153 148 154 149 static void annotate_browser__draw_current_jump(struct ui_browser *browser) ··· 164 155 struct annotation *notes = symbol__annotation(sym); 165 156 u8 pcnt_width = annotation__pcnt_width(notes); 166 157 int width; 158 + int diff = 0; 167 159 168 160 /* PLT symbols contain external offsets */ 169 161 if (strstr(sym->name, "@plt")) ··· 215 205 pcnt_width + 2 + notes->widths.addr + width, 216 206 from, to); 217 207 218 - if (is_fused(ab, cursor)) { 208 + diff = is_fused(ab, cursor); 209 + if (diff > 0) { 219 210 ui_browser__mark_fused(browser, 220 211 pcnt_width + 3 + notes->widths.addr + width, 221 - from - 1, 222 - to > from); 212 + from - diff, diff, to > from); 223 213 } 224 214 } 225 215