| Message ID | 20231012035111.676789-10-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | perf tools: Introduce data type profiling (v1) | expand |
On Wed, 11 Oct 2023 20:50:32 -0700 Namhyung Kim <namhyung@kernel.org> wrote: > The get_dwarf_regnum() returns a DWARF register number from a register > name string according to the psABI. Also add two pseudo encodings of > DWARF_REG_PC which is a register that are used by PC-relative addressing > and DWARF_REG_FB which is a frame base register. They need to be > handled in a special way. > I have some comments below. > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/arch/x86/util/dwarf-regs.c | 38 +++++++++++++++++++++++++++ > tools/perf/util/dwarf-regs.c | 33 +++++++++++++++++++++++ > tools/perf/util/include/dwarf-regs.h | 11 ++++++++ > 3 files changed, 82 insertions(+) > > diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c > index 530934805710..79835b897cae 100644 > --- a/tools/perf/arch/x86/util/dwarf-regs.c > +++ b/tools/perf/arch/x86/util/dwarf-regs.c > @@ -113,3 +113,41 @@ int regs_query_register_offset(const char *name) > return roff->offset; > return -EINVAL; > } > + > +struct dwarf_regs_idx { > + const char *name; > + int idx; > +}; > + > +static const struct dwarf_regs_idx x86_regidx_table[] = { > + { "rax", 0 }, { "eax", 0 }, { "ax", 0 }, { "al", 0 }, > + { "rdx", 1 }, { "edx", 1 }, { "dx", 1 }, { "dl", 1 }, > + { "rcx", 2 }, { "ecx", 2 }, { "cx", 2 }, { "cl", 2 }, > + { "rbx", 3 }, { "edx", 3 }, { "bx", 3 }, { "bl", 3 }, > + { "rsi", 4 }, { "esi", 4 }, { "si", 4 }, { "sil", 4 }, > + { "rdi", 5 }, { "edi", 5 }, { "di", 5 }, { "dil", 5 }, > + { "rbp", 6 }, { "ebp", 6 }, { "bp", 6 }, { "bpl", 6 }, > + { "rsp", 7 }, { "esp", 7 }, { "sp", 7 }, { "spl", 7 }, > + { "r8", 8 }, { "r8d", 8 }, { "r8w", 8 }, { "r8b", 8 }, > + { "r9", 9 }, { "r9d", 9 }, { "r9w", 9 }, { "r9b", 9 }, > + { "r10", 10 }, { "r10d", 10 }, { "r10w", 10 }, { "r10b", 10 }, > + { "r11", 11 }, { "r11d", 11 }, { "r11w", 11 }, { "r11b", 11 }, > + { "r12", 12 }, { "r12d", 12 }, { "r12w", 12 }, { "r12b", 12 }, > + { "r13", 13 }, { "r13d", 13 }, { "r13w", 13 }, { "r13b", 13 }, > + { "r14", 14 }, { "r14d", 14 }, { "r14w", 14 }, { "r14b", 14 }, > + { "r15", 15 }, { "r15d", 15 }, { "r15w", 15 }, { "r15b", 15 }, > + { "rip", DWARF_REG_PC }, > +}; > + > +int get_arch_regnum(const char *name) > +{ > + unsigned int i; > + > + if (*name != '%') > + return -1; Isn't it better to return an error code? e.g. -EINVAL. > + > + for (i = 0; i < ARRAY_SIZE(x86_regidx_table); i++) > + if (!strcmp(x86_regidx_table[i].name, name + 1)) > + return x86_regidx_table[i].idx; > + return -1; And -ENOENT. > +} > diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c > index 69cfaa5953bf..28d786c7df55 100644 > --- a/tools/perf/util/dwarf-regs.c > +++ b/tools/perf/util/dwarf-regs.c > @@ -5,6 +5,8 @@ > * Written by: Masami Hiramatsu <mhiramat@kernel.org> > */ > > +#include <stdlib.h> > +#include <string.h> > #include <debug.h> > #include <dwarf-regs.h> > #include <elf.h> > @@ -68,3 +70,34 @@ const char *get_dwarf_regstr(unsigned int n, unsigned int machine) > } > return NULL; > } > + > +__weak int get_arch_regnum(const char *name __maybe_unused) > +{ > + return -1; And -EOPNOTSUPP. > +} > + > +/* Return DWARF register number from architecture register name */ > +int get_dwarf_regnum(const char *name, unsigned int machine) > +{ > + char *regname = strdup(name); > + int reg = -1; > + char *p; > + > + if (regname == NULL) > + return -1; Here, -EINVAL. > + > + /* For convenience, remove trailing characters */ > + p = strpbrk(regname, " ,)"); > + if (p) > + *p = '\0'; > + > + switch (machine) { > + case EM_NONE: /* Generic arch - use host arch */ > + reg = get_arch_regnum(regname); > + break; > + default: > + pr_err("ELF MACHINE %x is not supported.\n", machine); > + } > + free(regname); > + return reg; > +} > diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h > index 7d99a084e82d..b515f694f55e 100644 > --- a/tools/perf/util/include/dwarf-regs.h > +++ b/tools/perf/util/include/dwarf-regs.h > @@ -2,6 +2,9 @@ > #ifndef _PERF_DWARF_REGS_H_ > #define _PERF_DWARF_REGS_H_ > > +#define DWARF_REG_PC 0xd3af9c /* random number */ > +#define DWARF_REG_FB 0xd3affb /* random number */ > + Is this for other patch in the series? > #ifdef HAVE_DWARF_SUPPORT > const char *get_arch_regstr(unsigned int n); > /* > @@ -10,6 +13,14 @@ const char *get_arch_regstr(unsigned int n); > * machine: ELF machine signature (EM_*) > */ > const char *get_dwarf_regstr(unsigned int n, unsigned int machine); > + > +int get_arch_regnum(const char *name); > +/* > + * get_dwarf_regnum - Returns DWARF regnum from register name > + * name: architecture register name > + * machine: ELF machine signature (EM_*) > + */ > +int get_dwarf_regnum(const char *name, unsigned int machine); > #endif > > #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET > -- > 2.42.0.655.g421f12c284-goog > Thank you,
On Sun, Nov 5, 2023 at 1:36 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > On Wed, 11 Oct 2023 20:50:32 -0700 > Namhyung Kim <namhyung@kernel.org> wrote: > > > The get_dwarf_regnum() returns a DWARF register number from a register > > name string according to the psABI. Also add two pseudo encodings of > > DWARF_REG_PC which is a register that are used by PC-relative addressing > > and DWARF_REG_FB which is a frame base register. They need to be > > handled in a special way. > > > > I have some comments below. Sure! :) > > > Cc: Masami Hiramatsu <mhiramat@kernel.org> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/arch/x86/util/dwarf-regs.c | 38 +++++++++++++++++++++++++++ > > tools/perf/util/dwarf-regs.c | 33 +++++++++++++++++++++++ > > tools/perf/util/include/dwarf-regs.h | 11 ++++++++ > > 3 files changed, 82 insertions(+) > > > > diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c > > index 530934805710..79835b897cae 100644 > > --- a/tools/perf/arch/x86/util/dwarf-regs.c > > +++ b/tools/perf/arch/x86/util/dwarf-regs.c > > @@ -113,3 +113,41 @@ int regs_query_register_offset(const char *name) > > return roff->offset; > > return -EINVAL; > > } > > + > > +struct dwarf_regs_idx { > > + const char *name; > > + int idx; > > +}; > > + > > +static const struct dwarf_regs_idx x86_regidx_table[] = { > > + { "rax", 0 }, { "eax", 0 }, { "ax", 0 }, { "al", 0 }, > > + { "rdx", 1 }, { "edx", 1 }, { "dx", 1 }, { "dl", 1 }, > > + { "rcx", 2 }, { "ecx", 2 }, { "cx", 2 }, { "cl", 2 }, > > + { "rbx", 3 }, { "edx", 3 }, { "bx", 3 }, { "bl", 3 }, > > + { "rsi", 4 }, { "esi", 4 }, { "si", 4 }, { "sil", 4 }, > > + { "rdi", 5 }, { "edi", 5 }, { "di", 5 }, { "dil", 5 }, > > + { "rbp", 6 }, { "ebp", 6 }, { "bp", 6 }, { "bpl", 6 }, > > + { "rsp", 7 }, { "esp", 7 }, { "sp", 7 }, { "spl", 7 }, > > + { "r8", 8 }, { "r8d", 8 }, { "r8w", 8 }, { "r8b", 8 }, > > + { "r9", 9 }, { "r9d", 9 }, { "r9w", 9 }, { "r9b", 9 }, > > + { "r10", 10 }, { "r10d", 10 }, { "r10w", 10 }, { "r10b", 10 }, > > + { "r11", 11 }, { "r11d", 11 }, { "r11w", 11 }, { "r11b", 11 }, > > + { "r12", 12 }, { "r12d", 12 }, { "r12w", 12 }, { "r12b", 12 }, > > + { "r13", 13 }, { "r13d", 13 }, { "r13w", 13 }, { "r13b", 13 }, > > + { "r14", 14 }, { "r14d", 14 }, { "r14w", 14 }, { "r14b", 14 }, > > + { "r15", 15 }, { "r15d", 15 }, { "r15w", 15 }, { "r15b", 15 }, > > + { "rip", DWARF_REG_PC }, > > +}; > > + > > +int get_arch_regnum(const char *name) > > +{ > > + unsigned int i; > > + > > + if (*name != '%') > > + return -1; > > Isn't it better to return an error code? e.g. -EINVAL. Maybe.. any negative error code would work. > > > + > > + for (i = 0; i < ARRAY_SIZE(x86_regidx_table); i++) > > + if (!strcmp(x86_regidx_table[i].name, name + 1)) > > + return x86_regidx_table[i].idx; > > + return -1; > > And -ENOENT. > > > +} > > diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c > > index 69cfaa5953bf..28d786c7df55 100644 > > --- a/tools/perf/util/dwarf-regs.c > > +++ b/tools/perf/util/dwarf-regs.c > > @@ -5,6 +5,8 @@ > > * Written by: Masami Hiramatsu <mhiramat@kernel.org> > > */ > > > > +#include <stdlib.h> > > +#include <string.h> > > #include <debug.h> > > #include <dwarf-regs.h> > > #include <elf.h> > > @@ -68,3 +70,34 @@ const char *get_dwarf_regstr(unsigned int n, unsigned int machine) > > } > > return NULL; > > } > > + > > +__weak int get_arch_regnum(const char *name __maybe_unused) > > +{ > > + return -1; > > And -EOPNOTSUPP. Could be -ENOTSUP. > > > +} > > + > > +/* Return DWARF register number from architecture register name */ > > +int get_dwarf_regnum(const char *name, unsigned int machine) > > +{ > > + char *regname = strdup(name); > > + int reg = -1; > > + char *p; > > + > > + if (regname == NULL) > > + return -1; > > Here, -EINVAL. > > > + > > + /* For convenience, remove trailing characters */ > > + p = strpbrk(regname, " ,)"); > > + if (p) > > + *p = '\0'; > > + > > + switch (machine) { > > + case EM_NONE: /* Generic arch - use host arch */ > > + reg = get_arch_regnum(regname); > > + break; > > + default: > > + pr_err("ELF MACHINE %x is not supported.\n", machine); > > + } > > + free(regname); > > + return reg; > > +} > > diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h > > index 7d99a084e82d..b515f694f55e 100644 > > --- a/tools/perf/util/include/dwarf-regs.h > > +++ b/tools/perf/util/include/dwarf-regs.h > > @@ -2,6 +2,9 @@ > > #ifndef _PERF_DWARF_REGS_H_ > > #define _PERF_DWARF_REGS_H_ > > > > +#define DWARF_REG_PC 0xd3af9c /* random number */ > > +#define DWARF_REG_FB 0xd3affb /* random number */ > > + > > Is this for other patch in the series? Yes, they are to support global and local variables later. Thanks, Namhyung > > > #ifdef HAVE_DWARF_SUPPORT > > const char *get_arch_regstr(unsigned int n); > > /* > > @@ -10,6 +13,14 @@ const char *get_arch_regstr(unsigned int n); > > * machine: ELF machine signature (EM_*) > > */ > > const char *get_dwarf_regstr(unsigned int n, unsigned int machine); > > + > > +int get_arch_regnum(const char *name); > > +/* > > + * get_dwarf_regnum - Returns DWARF regnum from register name > > + * name: architecture register name > > + * machine: ELF machine signature (EM_*) > > + */ > > +int get_dwarf_regnum(const char *name, unsigned int machine); > > #endif > > > > #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET > > -- > > 2.42.0.655.g421f12c284-goog > > > > Thank you, > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c index 530934805710..79835b897cae 100644 --- a/tools/perf/arch/x86/util/dwarf-regs.c +++ b/tools/perf/arch/x86/util/dwarf-regs.c @@ -113,3 +113,41 @@ int regs_query_register_offset(const char *name) return roff->offset; return -EINVAL; } + +struct dwarf_regs_idx { + const char *name; + int idx; +}; + +static const struct dwarf_regs_idx x86_regidx_table[] = { + { "rax", 0 }, { "eax", 0 }, { "ax", 0 }, { "al", 0 }, + { "rdx", 1 }, { "edx", 1 }, { "dx", 1 }, { "dl", 1 }, + { "rcx", 2 }, { "ecx", 2 }, { "cx", 2 }, { "cl", 2 }, + { "rbx", 3 }, { "edx", 3 }, { "bx", 3 }, { "bl", 3 }, + { "rsi", 4 }, { "esi", 4 }, { "si", 4 }, { "sil", 4 }, + { "rdi", 5 }, { "edi", 5 }, { "di", 5 }, { "dil", 5 }, + { "rbp", 6 }, { "ebp", 6 }, { "bp", 6 }, { "bpl", 6 }, + { "rsp", 7 }, { "esp", 7 }, { "sp", 7 }, { "spl", 7 }, + { "r8", 8 }, { "r8d", 8 }, { "r8w", 8 }, { "r8b", 8 }, + { "r9", 9 }, { "r9d", 9 }, { "r9w", 9 }, { "r9b", 9 }, + { "r10", 10 }, { "r10d", 10 }, { "r10w", 10 }, { "r10b", 10 }, + { "r11", 11 }, { "r11d", 11 }, { "r11w", 11 }, { "r11b", 11 }, + { "r12", 12 }, { "r12d", 12 }, { "r12w", 12 }, { "r12b", 12 }, + { "r13", 13 }, { "r13d", 13 }, { "r13w", 13 }, { "r13b", 13 }, + { "r14", 14 }, { "r14d", 14 }, { "r14w", 14 }, { "r14b", 14 }, + { "r15", 15 }, { "r15d", 15 }, { "r15w", 15 }, { "r15b", 15 }, + { "rip", DWARF_REG_PC }, +}; + +int get_arch_regnum(const char *name) +{ + unsigned int i; + + if (*name != '%') + return -1; + + for (i = 0; i < ARRAY_SIZE(x86_regidx_table); i++) + if (!strcmp(x86_regidx_table[i].name, name + 1)) + return x86_regidx_table[i].idx; + return -1; +} diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c index 69cfaa5953bf..28d786c7df55 100644 --- a/tools/perf/util/dwarf-regs.c +++ b/tools/perf/util/dwarf-regs.c @@ -5,6 +5,8 @@ * Written by: Masami Hiramatsu <mhiramat@kernel.org> */ +#include <stdlib.h> +#include <string.h> #include <debug.h> #include <dwarf-regs.h> #include <elf.h> @@ -68,3 +70,34 @@ const char *get_dwarf_regstr(unsigned int n, unsigned int machine) } return NULL; } + +__weak int get_arch_regnum(const char *name __maybe_unused) +{ + return -1; +} + +/* Return DWARF register number from architecture register name */ +int get_dwarf_regnum(const char *name, unsigned int machine) +{ + char *regname = strdup(name); + int reg = -1; + char *p; + + if (regname == NULL) + return -1; + + /* For convenience, remove trailing characters */ + p = strpbrk(regname, " ,)"); + if (p) + *p = '\0'; + + switch (machine) { + case EM_NONE: /* Generic arch - use host arch */ + reg = get_arch_regnum(regname); + break; + default: + pr_err("ELF MACHINE %x is not supported.\n", machine); + } + free(regname); + return reg; +} diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h index 7d99a084e82d..b515f694f55e 100644 --- a/tools/perf/util/include/dwarf-regs.h +++ b/tools/perf/util/include/dwarf-regs.h @@ -2,6 +2,9 @@ #ifndef _PERF_DWARF_REGS_H_ #define _PERF_DWARF_REGS_H_ +#define DWARF_REG_PC 0xd3af9c /* random number */ +#define DWARF_REG_FB 0xd3affb /* random number */ + #ifdef HAVE_DWARF_SUPPORT const char *get_arch_regstr(unsigned int n); /* @@ -10,6 +13,14 @@ const char *get_arch_regstr(unsigned int n); * machine: ELF machine signature (EM_*) */ const char *get_dwarf_regstr(unsigned int n, unsigned int machine); + +int get_arch_regnum(const char *name); +/* + * get_dwarf_regnum - Returns DWARF regnum from register name + * name: architecture register name + * machine: ELF machine signature (EM_*) + */ +int get_dwarf_regnum(const char *name, unsigned int machine); #endif #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET
The get_dwarf_regnum() returns a DWARF register number from a register name string according to the psABI. Also add two pseudo encodings of DWARF_REG_PC which is a register that are used by PC-relative addressing and DWARF_REG_FB which is a frame base register. They need to be handled in a special way. Cc: Masami Hiramatsu <mhiramat@kernel.org> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/arch/x86/util/dwarf-regs.c | 38 +++++++++++++++++++++++++++ tools/perf/util/dwarf-regs.c | 33 +++++++++++++++++++++++ tools/perf/util/include/dwarf-regs.h | 11 ++++++++ 3 files changed, 82 insertions(+)