Skip to content

Commit 7efbcc8

Browse files
Ravi Bangoriaacmel
Ravi Bangoria
authored andcommitted
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 <[email protected]> Signed-off-by: Ravi Bangoria <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Kim Phillips <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Namhyung Kim <[email protected]> Link: https //lore.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 93ff9f1 commit 7efbcc8

File tree

3 files changed

+42
-17
lines changed

3 files changed

+42
-17
lines changed

tools/perf/ui/browser.c

+24-9
Original file line numberDiff line numberDiff line change
@@ -757,25 +757,40 @@ void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
757757
}
758758

759759
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)
761761
{
762-
unsigned int end_row;
762+
int end_row;
763763

764-
if (row >= browser->top_idx)
765-
end_row = row - browser->top_idx;
766-
else
764+
if (diff <= 0)
767765
return;
768766

769767
SLsmg_set_char_set(1);
770768

771769
if (arrow_down) {
770+
if (row + diff <= browser->top_idx)
771+
return;
772+
773+
end_row = row + diff - browser->top_idx;
772774
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);
777775
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+
}
778789
} else {
790+
if (row < browser->top_idx)
791+
return;
792+
793+
end_row = row - browser->top_idx;
779794
ui_browser__gotorc(browser, end_row, column - 1);
780795
SLsmg_write_char(SLSMG_LTEE_CHAR);
781796
ui_browser__gotorc(browser, end_row, column);

tools/perf/ui/browser.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ void ui_browser__write_graph(struct ui_browser *browser, int graph);
5151
void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
5252
u64 start, u64 end);
5353
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);
5555
void __ui_browser__show_title(struct ui_browser *browser, const char *title);
5656
void ui_browser__show_title(struct ui_browser *browser, const char *title);
5757
int ui_browser__show(struct ui_browser *browser, const char *title,

tools/perf/ui/browsers/annotate.c

+17-7
Original file line numberDiff line numberDiff line change
@@ -125,23 +125,32 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
125125
ab->selection = al;
126126
}
127127

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)
129129
{
130130
struct disasm_line *pos = list_prev_entry(cursor, al.node);
131131
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+
}
132139

133140
if (!pos)
134-
return false;
141+
return 0;
135142

136143
if (ins__is_lock(&pos->ins))
137144
name = pos->ops.locked.ins.name;
138145
else
139146
name = pos->ins.name;
140147

141148
if (!name || !cursor->ins.name)
142-
return false;
149+
return 0;
143150

144-
return ins__is_fused(ab->arch, name, cursor->ins.name);
151+
if (ins__is_fused(ab->arch, name, cursor->ins.name))
152+
return diff;
153+
return 0;
145154
}
146155

147156
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)
155164
struct annotation *notes = symbol__annotation(sym);
156165
u8 pcnt_width = annotation__pcnt_width(notes);
157166
int width;
167+
int diff = 0;
158168

159169
/* PLT symbols contain external offsets */
160170
if (strstr(sym->name, "@plt"))
@@ -205,11 +215,11 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
205215
pcnt_width + 2 + notes->widths.addr + width,
206216
from, to);
207217

208-
if (is_fused(ab, cursor)) {
218+
diff = is_fused(ab, cursor);
219+
if (diff > 0) {
209220
ui_browser__mark_fused(browser,
210221
pcnt_width + 3 + notes->widths.addr + width,
211-
from - 1,
212-
to > from);
222+
from - diff, diff, to > from);
213223
}
214224
}
215225

0 commit comments

Comments
 (0)