| Message ID | 20231012035111.676789-14-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | perf tools: Introduce data type profiling (v1) | expand |
Em Wed, Oct 11, 2023 at 08:50:36PM -0700, Namhyung Kim escreveu: > The annotate_get_insn_location() is to get the detailed information of > instruction locations like registers and offset. It has source and > target operands locations in an array. Each operand can have a > register and an offset. The offset is meaningful when mem_ref flag is > set. This needs to be enclosed in HAVE_DWARF_SUPPORT, as the build is failing in systems where this isn't available, see patch below. CC /tmp/build/perf/arch/x86/util/pmu.o util/annotate.c: In function 'extract_reg_offset': util/annotate.c:3537:24: error: implicit declaration of function 'get_dwarf_regnum' [-Werror=implicit-function-declaration] 3537 | op_loc->reg1 = get_dwarf_regnum(regname, 0); | ^~~~~~~~~~~~~~~~ CC /tmp/build/perf/tests/vmlinux-kallsyms.o I tested it with 'make NO_DWARF=1' - Arnaldo diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 9d653a1e84ce4889..b998c81c89be04fc 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -3486,6 +3486,7 @@ int annotate_check_args(struct annotation_options *args) return 0; } +#ifdef HAVE_DWARF_SUPPORT /* * Get register number and access offset from the given instruction. * It assumes AT&T x86 asm format like OFFSET(REG). Maybe it needs @@ -3591,3 +3592,4 @@ int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, return 0; } +#endif // HAVE_DWARF_SUPPORT diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 4adda492233d2742..484be346a279c481 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -437,6 +437,7 @@ int annotate_parse_percent_type(const struct option *opt, const char *_str, int annotate_check_args(struct annotation_options *args); +#ifdef HAVE_DWARF_SUPPORT /** * struct annotated_op_loc - Location info of instruction operand * @reg: Register in the operand @@ -473,4 +474,5 @@ struct annotated_insn_loc { int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, struct annotated_insn_loc *loc); +#endif /* HAVE_DWARF_SUPPORT */ #endif /* __PERF_ANNOTATE_H */
Hi Arnaldo, On Mon, Oct 23, 2023 at 9:38 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Wed, Oct 11, 2023 at 08:50:36PM -0700, Namhyung Kim escreveu: > > The annotate_get_insn_location() is to get the detailed information of > > instruction locations like registers and offset. It has source and > > target operands locations in an array. Each operand can have a > > register and an offset. The offset is meaningful when mem_ref flag is > > set. > > This needs to be enclosed in HAVE_DWARF_SUPPORT, as the build is failing > in systems where this isn't available, see patch below. Thanks for the test and the patch, will add it to v2. Namhyung > > CC /tmp/build/perf/arch/x86/util/pmu.o > util/annotate.c: In function 'extract_reg_offset': > util/annotate.c:3537:24: error: implicit declaration of function 'get_dwarf_regnum' [-Werror=implicit-function-declaration] > 3537 | op_loc->reg1 = get_dwarf_regnum(regname, 0); > | ^~~~~~~~~~~~~~~~ > CC /tmp/build/perf/tests/vmlinux-kallsyms.o > > I tested it with 'make NO_DWARF=1' > > - Arnaldo > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 9d653a1e84ce4889..b998c81c89be04fc 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -3486,6 +3486,7 @@ int annotate_check_args(struct annotation_options *args) > return 0; > } > > +#ifdef HAVE_DWARF_SUPPORT > /* > * Get register number and access offset from the given instruction. > * It assumes AT&T x86 asm format like OFFSET(REG). Maybe it needs > @@ -3591,3 +3592,4 @@ int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, > > return 0; > } > +#endif // HAVE_DWARF_SUPPORT > diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h > index 4adda492233d2742..484be346a279c481 100644 > --- a/tools/perf/util/annotate.h > +++ b/tools/perf/util/annotate.h > @@ -437,6 +437,7 @@ int annotate_parse_percent_type(const struct option *opt, const char *_str, > > int annotate_check_args(struct annotation_options *args); > > +#ifdef HAVE_DWARF_SUPPORT > /** > * struct annotated_op_loc - Location info of instruction operand > * @reg: Register in the operand > @@ -473,4 +474,5 @@ struct annotated_insn_loc { > int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, > struct annotated_insn_loc *loc); > > +#endif /* HAVE_DWARF_SUPPORT */ > #endif /* __PERF_ANNOTATE_H */
On Tue, Oct 24, 2023 at 12:10 PM Namhyung Kim <namhyung@kernel.org> wrote: > > Hi Arnaldo, > > On Mon, Oct 23, 2023 at 9:38 AM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Em Wed, Oct 11, 2023 at 08:50:36PM -0700, Namhyung Kim escreveu: > > > The annotate_get_insn_location() is to get the detailed information of > > > instruction locations like registers and offset. It has source and > > > target operands locations in an array. Each operand can have a > > > register and an offset. The offset is meaningful when mem_ref flag is > > > set. > > > > This needs to be enclosed in HAVE_DWARF_SUPPORT, as the build is failing > > in systems where this isn't available, see patch below. > > Thanks for the test and the patch, will add it to v2. Hmm.. I think this function can work without DWARF. If the only problem is get_dwarf_regnum() probably I can add a dummy function when libdw is not found. Maybe I need to use arch reg number here and convert to DWARF later. Thanks, Namhyung > > > > CC /tmp/build/perf/arch/x86/util/pmu.o > > util/annotate.c: In function 'extract_reg_offset': > > util/annotate.c:3537:24: error: implicit declaration of function 'get_dwarf_regnum' [-Werror=implicit-function-declaration] > > 3537 | op_loc->reg1 = get_dwarf_regnum(regname, 0); > > | ^~~~~~~~~~~~~~~~ > > CC /tmp/build/perf/tests/vmlinux-kallsyms.o > > > > I tested it with 'make NO_DWARF=1' > > > > - Arnaldo > > > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > > index 9d653a1e84ce4889..b998c81c89be04fc 100644 > > --- a/tools/perf/util/annotate.c > > +++ b/tools/perf/util/annotate.c > > @@ -3486,6 +3486,7 @@ int annotate_check_args(struct annotation_options *args) > > return 0; > > } > > > > +#ifdef HAVE_DWARF_SUPPORT > > /* > > * Get register number and access offset from the given instruction. > > * It assumes AT&T x86 asm format like OFFSET(REG). Maybe it needs > > @@ -3591,3 +3592,4 @@ int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, > > > > return 0; > > } > > +#endif // HAVE_DWARF_SUPPORT > > diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h > > index 4adda492233d2742..484be346a279c481 100644 > > --- a/tools/perf/util/annotate.h > > +++ b/tools/perf/util/annotate.h > > @@ -437,6 +437,7 @@ int annotate_parse_percent_type(const struct option *opt, const char *_str, > > > > int annotate_check_args(struct annotation_options *args); > > > > +#ifdef HAVE_DWARF_SUPPORT > > /** > > * struct annotated_op_loc - Location info of instruction operand > > * @reg: Register in the operand > > @@ -473,4 +474,5 @@ struct annotated_insn_loc { > > int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, > > struct annotated_insn_loc *loc); > > > > +#endif /* HAVE_DWARF_SUPPORT */ > > #endif /* __PERF_ANNOTATE_H */
Em Wed, Oct 25, 2023 at 10:26:32PM -0700, Namhyung Kim escreveu: > On Tue, Oct 24, 2023 at 12:10 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Mon, Oct 23, 2023 at 9:38 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > Em Wed, Oct 11, 2023 at 08:50:36PM -0700, Namhyung Kim escreveu: > > > > The annotate_get_insn_location() is to get the detailed information of > > > > instruction locations like registers and offset. It has source and > > > > target operands locations in an array. Each operand can have a > > > > register and an offset. The offset is meaningful when mem_ref flag is > > > > set. > > > This needs to be enclosed in HAVE_DWARF_SUPPORT, as the build is failing > > > in systems where this isn't available, see patch below. > > Thanks for the test and the patch, will add it to v2. > Hmm.. I think this function can work without DWARF. > If the only problem is get_dwarf_regnum() probably > I can add a dummy function when libdw is not found. > Maybe I need to use arch reg number here and > convert to DWARF later. That can be a possibility, but the interesting thing is to try to run 'make -C tools/perf build-test' regularly to catch these builds without some of the components, I bet this one: make_no_libelf := NO_LIBELF=1 As my container builds did, as that is one of the things built there. One other suggestion would be to add some warning when the required components to build the data profiling feature are not present. - Arnaldo
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 254cc9f224f4..9d653a1e84ce 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -31,6 +31,7 @@ #include "bpf-utils.h" #include "block-range.h" #include "string2.h" +#include "dwarf-regs.h" #include "util/event.h" #include "util/sharded_mutex.h" #include "arch/common.h" @@ -3484,3 +3485,109 @@ int annotate_check_args(struct annotation_options *args) } return 0; } + +/* + * Get register number and access offset from the given instruction. + * It assumes AT&T x86 asm format like OFFSET(REG). Maybe it needs + * to revisit the format when it handles different architecture. + * Fills @reg and @offset when return 0. + */ +static int extract_reg_offset(struct arch *arch, const char *str, + struct annotated_op_loc *op_loc) +{ + char *p; + char *regname; + + if (arch->objdump.register_char == 0) + return -1; + + /* + * It should start from offset, but it's possible to skip 0 + * in the asm. So 0(%rax) should be same as (%rax). + * + * However, it also start with a segment select register like + * %gs:0x18(%rbx). In that case it should skip the part. + */ + if (*str == arch->objdump.register_char) { + while (*str && !isdigit(*str) && + *str != arch->objdump.memory_ref_char) + str++; + } + + op_loc->offset = strtol(str, &p, 0); + + p = strchr(p, arch->objdump.register_char); + if (p == NULL) + return -1; + + regname = strdup(p); + if (regname == NULL) + return -1; + + op_loc->reg = get_dwarf_regnum(regname, 0); + free(regname); + return 0; +} + +/** + * annotate_get_insn_location - Get location of instruction + * @arch: the architecture info + * @dl: the target instruction + * @loc: a buffer to save the data + * + * Get detailed location info (register and offset) in the instruction. + * It needs both source and target operand and whether it accesses a + * memory location. The offset field is meaningful only when the + * corresponding mem flag is set. + * + * Some examples on x86: + * + * mov (%rax), %rcx # src_reg = rax, src_mem = 1, src_offset = 0 + * # dst_reg = rcx, dst_mem = 0 + * + * mov 0x18, %r8 # src_reg = -1, dst_reg = r8 + */ +int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, + struct annotated_insn_loc *loc) +{ + struct ins_operands *ops; + struct annotated_op_loc *op_loc; + int i; + + if (!strcmp(dl->ins.name, "lock")) + ops = dl->ops.locked.ops; + else + ops = &dl->ops; + + if (ops == NULL) + return -1; + + memset(loc, 0, sizeof(*loc)); + + for_each_insn_op_loc(loc, i, op_loc) { + const char *insn_str = ops->source.raw; + + if (i == INSN_OP_TARGET) + insn_str = ops->target.raw; + + /* Invalidate the register by default */ + op_loc->reg = -1; + + if (insn_str == NULL) + continue; + + if (strchr(insn_str, arch->objdump.memory_ref_char)) { + op_loc->mem_ref = true; + extract_reg_offset(arch, insn_str, op_loc); + } else { + char *s = strdup(insn_str); + + if (s) { + op_loc->reg = get_dwarf_regnum(s, 0); + free(s); + } + } + } + + return 0; +} diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index c74f8f10f705..4adda492233d 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -437,4 +437,40 @@ int annotate_parse_percent_type(const struct option *opt, const char *_str, int annotate_check_args(struct annotation_options *args); +/** + * struct annotated_op_loc - Location info of instruction operand + * @reg: Register in the operand + * @offset: Memory access offset in the operand + * @mem_ref: Whether the operand accesses memory + */ +struct annotated_op_loc { + int reg; + int offset; + bool mem_ref; +}; + +enum annotated_insn_ops { + INSN_OP_SOURCE = 0, + INSN_OP_TARGET = 1, + + INSN_OP_MAX, +}; + +/** + * struct annotated_insn_loc - Location info of instruction + * @ops: Array of location info for source and target operands + */ +struct annotated_insn_loc { + struct annotated_op_loc ops[INSN_OP_MAX]; +}; + +#define for_each_insn_op_loc(insn_loc, i, op_loc) \ + for (i = INSN_OP_SOURCE, op_loc = &(insn_loc)->ops[i]; \ + i < INSN_OP_MAX; \ + i++, op_loc++) + +/* Get detailed location info in the instruction */ +int annotate_get_insn_location(struct arch *arch, struct disasm_line *dl, + struct annotated_insn_loc *loc); + #endif /* __PERF_ANNOTATE_H */
The annotate_get_insn_location() is to get the detailed information of instruction locations like registers and offset. It has source and target operands locations in an array. Each operand can have a register and an offset. The offset is meaningful when mem_ref flag is set. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/annotate.c | 107 +++++++++++++++++++++++++++++++++++++ tools/perf/util/annotate.h | 36 +++++++++++++ 2 files changed, 143 insertions(+)