* [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally @ 2023-05-24 5:28 Namhyung Kim 2023-05-24 5:28 ` [PATCH 2/2] perf annotate: Remove x86 instructions with suffix Namhyung Kim 2023-05-24 6:11 ` [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally Adrian Hunter 0 siblings, 2 replies; 7+ messages in thread From: Namhyung Kim @ 2023-05-24 5:28 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Jiri Olsa Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Andi Kleen, Masami Hiramatsu, Kan Liang Most of x86 instructions can have size suffix like b, w, l or q. Instead of adding all these instructions in the table, we can handle them in a general way. For example, it can try to find an instruction as is. If not found, it'd try again without the suffix if it's one of the allowed suffixes. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/annotate.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index b708bbc49c9e..7f05f2a2aa83 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -70,6 +70,7 @@ struct arch { struct ins_ops *(*associate_instruction_ops)(struct arch *arch, const char *name); bool sorted_instructions; bool initialized; + const char *insn_suffix; void *priv; unsigned int model; unsigned int family; @@ -179,6 +180,7 @@ static struct arch architectures[] = { .init = x86__annotate_init, .instructions = x86__instructions, .nr_instructions = ARRAY_SIZE(x86__instructions), + .insn_suffix = "bwlq", .objdump = { .comment_char = '#', }, @@ -720,6 +722,26 @@ static struct ins_ops *__ins__find(struct arch *arch, const char *name) } ins = bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); + if (ins) + return ins->ops; + + if (arch->insn_suffix) { + char tmp[32]; + char suffix; + size_t len = strlen(name); + + if (len == 0 || len >= sizeof(tmp)) + return NULL; + + suffix = name[len - 1]; + if (strchr(arch->insn_suffix, suffix) == NULL) + return NULL; + + strcpy(tmp, name); + tmp[len - 1] = '\0'; /* remove the suffix and check again */ + + ins = bsearch(tmp, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); + } return ins ? ins->ops : NULL; } -- 2.40.1.698.g37aff9b760-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] perf annotate: Remove x86 instructions with suffix 2023-05-24 5:28 [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally Namhyung Kim @ 2023-05-24 5:28 ` Namhyung Kim 2023-05-24 6:14 ` Adrian Hunter 2023-05-24 6:11 ` [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally Adrian Hunter 1 sibling, 1 reply; 7+ messages in thread From: Namhyung Kim @ 2023-05-24 5:28 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Jiri Olsa Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Andi Kleen, Masami Hiramatsu, Kan Liang Not the suffix is handled in the general code. Let's get rid of them. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/arch/x86/annotate/instructions.c | 44 +-------------------- 1 file changed, 2 insertions(+), 42 deletions(-) diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c index 5c7bec25fee4..714fd8784d99 100644 --- a/tools/perf/arch/x86/annotate/instructions.c +++ b/tools/perf/arch/x86/annotate/instructions.c @@ -1,48 +1,29 @@ // SPDX-License-Identifier: GPL-2.0 static struct ins x86__instructions[] = { { .name = "adc", .ops = &mov_ops, }, - { .name = "adcb", .ops = &mov_ops, }, - { .name = "adcl", .ops = &mov_ops, }, { .name = "add", .ops = &mov_ops, }, - { .name = "addl", .ops = &mov_ops, }, - { .name = "addq", .ops = &mov_ops, }, { .name = "addsd", .ops = &mov_ops, }, - { .name = "addw", .ops = &mov_ops, }, { .name = "and", .ops = &mov_ops, }, - { .name = "andb", .ops = &mov_ops, }, - { .name = "andl", .ops = &mov_ops, }, { .name = "andpd", .ops = &mov_ops, }, { .name = "andps", .ops = &mov_ops, }, - { .name = "andq", .ops = &mov_ops, }, - { .name = "andw", .ops = &mov_ops, }, { .name = "bsr", .ops = &mov_ops, }, { .name = "bt", .ops = &mov_ops, }, { .name = "btr", .ops = &mov_ops, }, { .name = "bts", .ops = &mov_ops, }, - { .name = "btsq", .ops = &mov_ops, }, { .name = "call", .ops = &call_ops, }, - { .name = "callq", .ops = &call_ops, }, { .name = "cmovbe", .ops = &mov_ops, }, { .name = "cmove", .ops = &mov_ops, }, { .name = "cmovae", .ops = &mov_ops, }, { .name = "cmp", .ops = &mov_ops, }, - { .name = "cmpb", .ops = &mov_ops, }, - { .name = "cmpl", .ops = &mov_ops, }, - { .name = "cmpq", .ops = &mov_ops, }, - { .name = "cmpw", .ops = &mov_ops, }, { .name = "cmpxch", .ops = &mov_ops, }, { .name = "cmpxchg", .ops = &mov_ops, }, { .name = "cs", .ops = &mov_ops, }, { .name = "dec", .ops = &dec_ops, }, - { .name = "decl", .ops = &dec_ops, }, - { .name = "decq", .ops = &dec_ops, }, { .name = "divsd", .ops = &mov_ops, }, { .name = "divss", .ops = &mov_ops, }, { .name = "gs", .ops = &mov_ops, }, { .name = "imul", .ops = &mov_ops, }, { .name = "inc", .ops = &dec_ops, }, - { .name = "incl", .ops = &dec_ops, }, - { .name = "incq", .ops = &dec_ops, }, { .name = "ja", .ops = &jump_ops, }, { .name = "jae", .ops = &jump_ops, }, { .name = "jb", .ops = &jump_ops, }, @@ -56,7 +37,6 @@ static struct ins x86__instructions[] = { { .name = "jl", .ops = &jump_ops, }, { .name = "jle", .ops = &jump_ops, }, { .name = "jmp", .ops = &jump_ops, }, - { .name = "jmpq", .ops = &jump_ops, }, { .name = "jna", .ops = &jump_ops, }, { .name = "jnae", .ops = &jump_ops, }, { .name = "jnb", .ops = &jump_ops, }, @@ -83,49 +63,31 @@ static struct ins x86__instructions[] = { { .name = "mov", .ops = &mov_ops, }, { .name = "movapd", .ops = &mov_ops, }, { .name = "movaps", .ops = &mov_ops, }, - { .name = "movb", .ops = &mov_ops, }, { .name = "movdqa", .ops = &mov_ops, }, { .name = "movdqu", .ops = &mov_ops, }, - { .name = "movl", .ops = &mov_ops, }, - { .name = "movq", .ops = &mov_ops, }, { .name = "movsd", .ops = &mov_ops, }, { .name = "movslq", .ops = &mov_ops, }, { .name = "movss", .ops = &mov_ops, }, { .name = "movupd", .ops = &mov_ops, }, { .name = "movups", .ops = &mov_ops, }, - { .name = "movw", .ops = &mov_ops, }, { .name = "movzbl", .ops = &mov_ops, }, { .name = "movzwl", .ops = &mov_ops, }, { .name = "mulsd", .ops = &mov_ops, }, { .name = "mulss", .ops = &mov_ops, }, { .name = "nop", .ops = &nop_ops, }, - { .name = "nopl", .ops = &nop_ops, }, - { .name = "nopw", .ops = &nop_ops, }, { .name = "or", .ops = &mov_ops, }, - { .name = "orb", .ops = &mov_ops, }, - { .name = "orl", .ops = &mov_ops, }, { .name = "orps", .ops = &mov_ops, }, - { .name = "orq", .ops = &mov_ops, }, { .name = "pand", .ops = &mov_ops, }, { .name = "paddq", .ops = &mov_ops, }, { .name = "pcmpeqb", .ops = &mov_ops, }, { .name = "por", .ops = &mov_ops, }, - { .name = "rclb", .ops = &mov_ops, }, - { .name = "rcll", .ops = &mov_ops, }, + { .name = "rcl", .ops = &mov_ops, }, { .name = "ret", .ops = &ret_ops, }, - { .name = "retq", .ops = &ret_ops, }, { .name = "sbb", .ops = &mov_ops, }, - { .name = "sbbl", .ops = &mov_ops, }, { .name = "sete", .ops = &mov_ops, }, { .name = "sub", .ops = &mov_ops, }, - { .name = "subl", .ops = &mov_ops, }, - { .name = "subq", .ops = &mov_ops, }, { .name = "subsd", .ops = &mov_ops, }, - { .name = "subw", .ops = &mov_ops, }, { .name = "test", .ops = &mov_ops, }, - { .name = "testb", .ops = &mov_ops, }, - { .name = "testl", .ops = &mov_ops, }, - { .name = "testq", .ops = &mov_ops, }, { .name = "tzcnt", .ops = &mov_ops, }, { .name = "ucomisd", .ops = &mov_ops, }, { .name = "ucomiss", .ops = &mov_ops, }, @@ -139,11 +101,9 @@ static struct ins x86__instructions[] = { { .name = "vsubsd", .ops = &mov_ops, }, { .name = "vucomisd", .ops = &mov_ops, }, { .name = "xadd", .ops = &mov_ops, }, - { .name = "xbeginl", .ops = &jump_ops, }, - { .name = "xbeginq", .ops = &jump_ops, }, + { .name = "xbegin", .ops = &jump_ops, }, { .name = "xchg", .ops = &mov_ops, }, { .name = "xor", .ops = &mov_ops, }, - { .name = "xorb", .ops = &mov_ops, }, { .name = "xorpd", .ops = &mov_ops, }, { .name = "xorps", .ops = &mov_ops, }, }; -- 2.40.1.698.g37aff9b760-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] perf annotate: Remove x86 instructions with suffix 2023-05-24 5:28 ` [PATCH 2/2] perf annotate: Remove x86 instructions with suffix Namhyung Kim @ 2023-05-24 6:14 ` Adrian Hunter 2023-05-24 16:11 ` Namhyung Kim 0 siblings, 1 reply; 7+ messages in thread From: Adrian Hunter @ 2023-05-24 6:14 UTC (permalink / raw) To: Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Andi Kleen, Masami Hiramatsu, Kan Liang On 24/05/23 08:28, Namhyung Kim wrote: > Not the suffix is handled in the general code. Let's get rid of them. Not -> Now ? > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/arch/x86/annotate/instructions.c | 44 +-------------------- > 1 file changed, 2 insertions(+), 42 deletions(-) > > diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c > index 5c7bec25fee4..714fd8784d99 100644 > --- a/tools/perf/arch/x86/annotate/instructions.c > +++ b/tools/perf/arch/x86/annotate/instructions.c > @@ -1,48 +1,29 @@ > // SPDX-License-Identifier: GPL-2.0 Could probably use a comment about how the table works with suffixes. > static struct ins x86__instructions[] = { > { .name = "adc", .ops = &mov_ops, }, > - { .name = "adcb", .ops = &mov_ops, }, > - { .name = "adcl", .ops = &mov_ops, }, > { .name = "add", .ops = &mov_ops, }, > - { .name = "addl", .ops = &mov_ops, }, > - { .name = "addq", .ops = &mov_ops, }, > { .name = "addsd", .ops = &mov_ops, }, > - { .name = "addw", .ops = &mov_ops, }, > { .name = "and", .ops = &mov_ops, }, > - { .name = "andb", .ops = &mov_ops, }, > - { .name = "andl", .ops = &mov_ops, }, > { .name = "andpd", .ops = &mov_ops, }, > { .name = "andps", .ops = &mov_ops, }, > - { .name = "andq", .ops = &mov_ops, }, > - { .name = "andw", .ops = &mov_ops, }, > { .name = "bsr", .ops = &mov_ops, }, > { .name = "bt", .ops = &mov_ops, }, > { .name = "btr", .ops = &mov_ops, }, > { .name = "bts", .ops = &mov_ops, }, > - { .name = "btsq", .ops = &mov_ops, }, > { .name = "call", .ops = &call_ops, }, > - { .name = "callq", .ops = &call_ops, }, > { .name = "cmovbe", .ops = &mov_ops, }, > { .name = "cmove", .ops = &mov_ops, }, > { .name = "cmovae", .ops = &mov_ops, }, > { .name = "cmp", .ops = &mov_ops, }, > - { .name = "cmpb", .ops = &mov_ops, }, > - { .name = "cmpl", .ops = &mov_ops, }, > - { .name = "cmpq", .ops = &mov_ops, }, > - { .name = "cmpw", .ops = &mov_ops, }, > { .name = "cmpxch", .ops = &mov_ops, }, > { .name = "cmpxchg", .ops = &mov_ops, }, > { .name = "cs", .ops = &mov_ops, }, > { .name = "dec", .ops = &dec_ops, }, > - { .name = "decl", .ops = &dec_ops, }, > - { .name = "decq", .ops = &dec_ops, }, > { .name = "divsd", .ops = &mov_ops, }, > { .name = "divss", .ops = &mov_ops, }, > { .name = "gs", .ops = &mov_ops, }, > { .name = "imul", .ops = &mov_ops, }, > { .name = "inc", .ops = &dec_ops, }, > - { .name = "incl", .ops = &dec_ops, }, > - { .name = "incq", .ops = &dec_ops, }, > { .name = "ja", .ops = &jump_ops, }, > { .name = "jae", .ops = &jump_ops, }, > { .name = "jb", .ops = &jump_ops, }, > @@ -56,7 +37,6 @@ static struct ins x86__instructions[] = { > { .name = "jl", .ops = &jump_ops, }, > { .name = "jle", .ops = &jump_ops, }, > { .name = "jmp", .ops = &jump_ops, }, > - { .name = "jmpq", .ops = &jump_ops, }, > { .name = "jna", .ops = &jump_ops, }, > { .name = "jnae", .ops = &jump_ops, }, > { .name = "jnb", .ops = &jump_ops, }, > @@ -83,49 +63,31 @@ static struct ins x86__instructions[] = { > { .name = "mov", .ops = &mov_ops, }, > { .name = "movapd", .ops = &mov_ops, }, > { .name = "movaps", .ops = &mov_ops, }, > - { .name = "movb", .ops = &mov_ops, }, > { .name = "movdqa", .ops = &mov_ops, }, > { .name = "movdqu", .ops = &mov_ops, }, > - { .name = "movl", .ops = &mov_ops, }, > - { .name = "movq", .ops = &mov_ops, }, > { .name = "movsd", .ops = &mov_ops, }, > { .name = "movslq", .ops = &mov_ops, }, > { .name = "movss", .ops = &mov_ops, }, > { .name = "movupd", .ops = &mov_ops, }, > { .name = "movups", .ops = &mov_ops, }, > - { .name = "movw", .ops = &mov_ops, }, > { .name = "movzbl", .ops = &mov_ops, }, > { .name = "movzwl", .ops = &mov_ops, }, > { .name = "mulsd", .ops = &mov_ops, }, > { .name = "mulss", .ops = &mov_ops, }, > { .name = "nop", .ops = &nop_ops, }, > - { .name = "nopl", .ops = &nop_ops, }, > - { .name = "nopw", .ops = &nop_ops, }, > { .name = "or", .ops = &mov_ops, }, > - { .name = "orb", .ops = &mov_ops, }, > - { .name = "orl", .ops = &mov_ops, }, > { .name = "orps", .ops = &mov_ops, }, > - { .name = "orq", .ops = &mov_ops, }, > { .name = "pand", .ops = &mov_ops, }, > { .name = "paddq", .ops = &mov_ops, }, > { .name = "pcmpeqb", .ops = &mov_ops, }, > { .name = "por", .ops = &mov_ops, }, > - { .name = "rclb", .ops = &mov_ops, }, > - { .name = "rcll", .ops = &mov_ops, }, > + { .name = "rcl", .ops = &mov_ops, }, > { .name = "ret", .ops = &ret_ops, }, > - { .name = "retq", .ops = &ret_ops, }, > { .name = "sbb", .ops = &mov_ops, }, > - { .name = "sbbl", .ops = &mov_ops, }, > { .name = "sete", .ops = &mov_ops, }, > { .name = "sub", .ops = &mov_ops, }, > - { .name = "subl", .ops = &mov_ops, }, > - { .name = "subq", .ops = &mov_ops, }, > { .name = "subsd", .ops = &mov_ops, }, > - { .name = "subw", .ops = &mov_ops, }, > { .name = "test", .ops = &mov_ops, }, > - { .name = "testb", .ops = &mov_ops, }, > - { .name = "testl", .ops = &mov_ops, }, > - { .name = "testq", .ops = &mov_ops, }, > { .name = "tzcnt", .ops = &mov_ops, }, > { .name = "ucomisd", .ops = &mov_ops, }, > { .name = "ucomiss", .ops = &mov_ops, }, > @@ -139,11 +101,9 @@ static struct ins x86__instructions[] = { > { .name = "vsubsd", .ops = &mov_ops, }, > { .name = "vucomisd", .ops = &mov_ops, }, > { .name = "xadd", .ops = &mov_ops, }, > - { .name = "xbeginl", .ops = &jump_ops, }, > - { .name = "xbeginq", .ops = &jump_ops, }, > + { .name = "xbegin", .ops = &jump_ops, }, > { .name = "xchg", .ops = &mov_ops, }, > { .name = "xor", .ops = &mov_ops, }, > - { .name = "xorb", .ops = &mov_ops, }, > { .name = "xorpd", .ops = &mov_ops, }, > { .name = "xorps", .ops = &mov_ops, }, > }; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] perf annotate: Remove x86 instructions with suffix 2023-05-24 6:14 ` Adrian Hunter @ 2023-05-24 16:11 ` Namhyung Kim 0 siblings, 0 replies; 7+ messages in thread From: Namhyung Kim @ 2023-05-24 16:11 UTC (permalink / raw) To: Adrian Hunter Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Andi Kleen, Masami Hiramatsu, Kan Liang On Tue, May 23, 2023 at 11:15 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 24/05/23 08:28, Namhyung Kim wrote: > > Not the suffix is handled in the general code. Let's get rid of them. > > Not -> Now ? Correct, will fix. > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/arch/x86/annotate/instructions.c | 44 +-------------------- > > 1 file changed, 2 insertions(+), 42 deletions(-) > > > > diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c > > index 5c7bec25fee4..714fd8784d99 100644 > > --- a/tools/perf/arch/x86/annotate/instructions.c > > +++ b/tools/perf/arch/x86/annotate/instructions.c > > @@ -1,48 +1,29 @@ > > // SPDX-License-Identifier: GPL-2.0 > > Could probably use a comment about how the table works > with suffixes. Ok, I will add a comment. Thanks for your review! Namhyung > > > static struct ins x86__instructions[] = { > > { .name = "adc", .ops = &mov_ops, }, > > - { .name = "adcb", .ops = &mov_ops, }, > > - { .name = "adcl", .ops = &mov_ops, }, > > { .name = "add", .ops = &mov_ops, }, > > - { .name = "addl", .ops = &mov_ops, }, > > - { .name = "addq", .ops = &mov_ops, }, > > { .name = "addsd", .ops = &mov_ops, }, > > - { .name = "addw", .ops = &mov_ops, }, > > { .name = "and", .ops = &mov_ops, }, > > - { .name = "andb", .ops = &mov_ops, }, > > - { .name = "andl", .ops = &mov_ops, }, > > { .name = "andpd", .ops = &mov_ops, }, > > { .name = "andps", .ops = &mov_ops, }, > > - { .name = "andq", .ops = &mov_ops, }, > > - { .name = "andw", .ops = &mov_ops, }, > > { .name = "bsr", .ops = &mov_ops, }, > > { .name = "bt", .ops = &mov_ops, }, > > { .name = "btr", .ops = &mov_ops, }, > > { .name = "bts", .ops = &mov_ops, }, > > - { .name = "btsq", .ops = &mov_ops, }, > > { .name = "call", .ops = &call_ops, }, > > - { .name = "callq", .ops = &call_ops, }, > > { .name = "cmovbe", .ops = &mov_ops, }, > > { .name = "cmove", .ops = &mov_ops, }, > > { .name = "cmovae", .ops = &mov_ops, }, > > { .name = "cmp", .ops = &mov_ops, }, > > - { .name = "cmpb", .ops = &mov_ops, }, > > - { .name = "cmpl", .ops = &mov_ops, }, > > - { .name = "cmpq", .ops = &mov_ops, }, > > - { .name = "cmpw", .ops = &mov_ops, }, > > { .name = "cmpxch", .ops = &mov_ops, }, > > { .name = "cmpxchg", .ops = &mov_ops, }, > > { .name = "cs", .ops = &mov_ops, }, > > { .name = "dec", .ops = &dec_ops, }, > > - { .name = "decl", .ops = &dec_ops, }, > > - { .name = "decq", .ops = &dec_ops, }, > > { .name = "divsd", .ops = &mov_ops, }, > > { .name = "divss", .ops = &mov_ops, }, > > { .name = "gs", .ops = &mov_ops, }, > > { .name = "imul", .ops = &mov_ops, }, > > { .name = "inc", .ops = &dec_ops, }, > > - { .name = "incl", .ops = &dec_ops, }, > > - { .name = "incq", .ops = &dec_ops, }, > > { .name = "ja", .ops = &jump_ops, }, > > { .name = "jae", .ops = &jump_ops, }, > > { .name = "jb", .ops = &jump_ops, }, > > @@ -56,7 +37,6 @@ static struct ins x86__instructions[] = { > > { .name = "jl", .ops = &jump_ops, }, > > { .name = "jle", .ops = &jump_ops, }, > > { .name = "jmp", .ops = &jump_ops, }, > > - { .name = "jmpq", .ops = &jump_ops, }, > > { .name = "jna", .ops = &jump_ops, }, > > { .name = "jnae", .ops = &jump_ops, }, > > { .name = "jnb", .ops = &jump_ops, }, > > @@ -83,49 +63,31 @@ static struct ins x86__instructions[] = { > > { .name = "mov", .ops = &mov_ops, }, > > { .name = "movapd", .ops = &mov_ops, }, > > { .name = "movaps", .ops = &mov_ops, }, > > - { .name = "movb", .ops = &mov_ops, }, > > { .name = "movdqa", .ops = &mov_ops, }, > > { .name = "movdqu", .ops = &mov_ops, }, > > - { .name = "movl", .ops = &mov_ops, }, > > - { .name = "movq", .ops = &mov_ops, }, > > { .name = "movsd", .ops = &mov_ops, }, > > { .name = "movslq", .ops = &mov_ops, }, > > { .name = "movss", .ops = &mov_ops, }, > > { .name = "movupd", .ops = &mov_ops, }, > > { .name = "movups", .ops = &mov_ops, }, > > - { .name = "movw", .ops = &mov_ops, }, > > { .name = "movzbl", .ops = &mov_ops, }, > > { .name = "movzwl", .ops = &mov_ops, }, > > { .name = "mulsd", .ops = &mov_ops, }, > > { .name = "mulss", .ops = &mov_ops, }, > > { .name = "nop", .ops = &nop_ops, }, > > - { .name = "nopl", .ops = &nop_ops, }, > > - { .name = "nopw", .ops = &nop_ops, }, > > { .name = "or", .ops = &mov_ops, }, > > - { .name = "orb", .ops = &mov_ops, }, > > - { .name = "orl", .ops = &mov_ops, }, > > { .name = "orps", .ops = &mov_ops, }, > > - { .name = "orq", .ops = &mov_ops, }, > > { .name = "pand", .ops = &mov_ops, }, > > { .name = "paddq", .ops = &mov_ops, }, > > { .name = "pcmpeqb", .ops = &mov_ops, }, > > { .name = "por", .ops = &mov_ops, }, > > - { .name = "rclb", .ops = &mov_ops, }, > > - { .name = "rcll", .ops = &mov_ops, }, > > + { .name = "rcl", .ops = &mov_ops, }, > > { .name = "ret", .ops = &ret_ops, }, > > - { .name = "retq", .ops = &ret_ops, }, > > { .name = "sbb", .ops = &mov_ops, }, > > - { .name = "sbbl", .ops = &mov_ops, }, > > { .name = "sete", .ops = &mov_ops, }, > > { .name = "sub", .ops = &mov_ops, }, > > - { .name = "subl", .ops = &mov_ops, }, > > - { .name = "subq", .ops = &mov_ops, }, > > { .name = "subsd", .ops = &mov_ops, }, > > - { .name = "subw", .ops = &mov_ops, }, > > { .name = "test", .ops = &mov_ops, }, > > - { .name = "testb", .ops = &mov_ops, }, > > - { .name = "testl", .ops = &mov_ops, }, > > - { .name = "testq", .ops = &mov_ops, }, > > { .name = "tzcnt", .ops = &mov_ops, }, > > { .name = "ucomisd", .ops = &mov_ops, }, > > { .name = "ucomiss", .ops = &mov_ops, }, > > @@ -139,11 +101,9 @@ static struct ins x86__instructions[] = { > > { .name = "vsubsd", .ops = &mov_ops, }, > > { .name = "vucomisd", .ops = &mov_ops, }, > > { .name = "xadd", .ops = &mov_ops, }, > > - { .name = "xbeginl", .ops = &jump_ops, }, > > - { .name = "xbeginq", .ops = &jump_ops, }, > > + { .name = "xbegin", .ops = &jump_ops, }, > > { .name = "xchg", .ops = &mov_ops, }, > > { .name = "xor", .ops = &mov_ops, }, > > - { .name = "xorb", .ops = &mov_ops, }, > > { .name = "xorpd", .ops = &mov_ops, }, > > { .name = "xorps", .ops = &mov_ops, }, > > }; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally 2023-05-24 5:28 [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally Namhyung Kim 2023-05-24 5:28 ` [PATCH 2/2] perf annotate: Remove x86 instructions with suffix Namhyung Kim @ 2023-05-24 6:11 ` Adrian Hunter 2023-05-24 16:10 ` Namhyung Kim 1 sibling, 1 reply; 7+ messages in thread From: Adrian Hunter @ 2023-05-24 6:11 UTC (permalink / raw) To: Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Andi Kleen, Masami Hiramatsu, Kan Liang On 24/05/23 08:28, Namhyung Kim wrote: > Most of x86 instructions can have size suffix like b, w, l or q. (AT&T mnemonics) > Instead of adding all these instructions in the table, we can handle > them in a general way. For example, it can try to find an instruction > as is. If not found, it'd try again without the suffix if it's one of > the allowed suffixes. I guess it might be possible that xyz is in the table but xyz<suffix> is a completely different instruction? > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/annotate.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index b708bbc49c9e..7f05f2a2aa83 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -70,6 +70,7 @@ struct arch { > struct ins_ops *(*associate_instruction_ops)(struct arch *arch, const char *name); > bool sorted_instructions; > bool initialized; > + const char *insn_suffix; > void *priv; > unsigned int model; > unsigned int family; > @@ -179,6 +180,7 @@ static struct arch architectures[] = { > .init = x86__annotate_init, > .instructions = x86__instructions, > .nr_instructions = ARRAY_SIZE(x86__instructions), > + .insn_suffix = "bwlq", > .objdump = { > .comment_char = '#', > }, > @@ -720,6 +722,26 @@ static struct ins_ops *__ins__find(struct arch *arch, const char *name) > } > > ins = bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); > + if (ins) > + return ins->ops; > + > + if (arch->insn_suffix) { > + char tmp[32]; > + char suffix; > + size_t len = strlen(name); > + > + if (len == 0 || len >= sizeof(tmp)) > + return NULL; > + > + suffix = name[len - 1]; > + if (strchr(arch->insn_suffix, suffix) == NULL) > + return NULL; > + > + strcpy(tmp, name); > + tmp[len - 1] = '\0'; /* remove the suffix and check again */ > + > + ins = bsearch(tmp, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); > + } > return ins ? ins->ops : NULL; > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally 2023-05-24 6:11 ` [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally Adrian Hunter @ 2023-05-24 16:10 ` Namhyung Kim 2023-05-24 17:41 ` Adrian Hunter 0 siblings, 1 reply; 7+ messages in thread From: Namhyung Kim @ 2023-05-24 16:10 UTC (permalink / raw) To: Adrian Hunter Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Andi Kleen, Masami Hiramatsu, Kan Liang Hi Adrian, On Tue, May 23, 2023 at 11:11 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 24/05/23 08:28, Namhyung Kim wrote: > > Most of x86 instructions can have size suffix like b, w, l or q. > > (AT&T mnemonics) Right, will update. > > > Instead of adding all these instructions in the table, we can handle > > them in a general way. For example, it can try to find an instruction > > as is. If not found, it'd try again without the suffix if it's one of > > the allowed suffixes. > > I guess it might be possible that xyz is in the table but xyz<suffix> > is a completely different instruction? Then xyz<suffix> should be in the table too. The match without suffix is a fallback so it should find the correct instruction first. Thanks, Namhyung > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/util/annotate.c | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > > index b708bbc49c9e..7f05f2a2aa83 100644 > > --- a/tools/perf/util/annotate.c > > +++ b/tools/perf/util/annotate.c > > @@ -70,6 +70,7 @@ struct arch { > > struct ins_ops *(*associate_instruction_ops)(struct arch *arch, const char *name); > > bool sorted_instructions; > > bool initialized; > > + const char *insn_suffix; > > void *priv; > > unsigned int model; > > unsigned int family; > > @@ -179,6 +180,7 @@ static struct arch architectures[] = { > > .init = x86__annotate_init, > > .instructions = x86__instructions, > > .nr_instructions = ARRAY_SIZE(x86__instructions), > > + .insn_suffix = "bwlq", > > .objdump = { > > .comment_char = '#', > > }, > > @@ -720,6 +722,26 @@ static struct ins_ops *__ins__find(struct arch *arch, const char *name) > > } > > > > ins = bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); > > + if (ins) > > + return ins->ops; > > + > > + if (arch->insn_suffix) { > > + char tmp[32]; > > + char suffix; > > + size_t len = strlen(name); > > + > > + if (len == 0 || len >= sizeof(tmp)) > > + return NULL; > > + > > + suffix = name[len - 1]; > > + if (strchr(arch->insn_suffix, suffix) == NULL) > > + return NULL; > > + > > + strcpy(tmp, name); > > + tmp[len - 1] = '\0'; /* remove the suffix and check again */ > > + > > + ins = bsearch(tmp, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); > > + } > > return ins ? ins->ops : NULL; > > } > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally 2023-05-24 16:10 ` Namhyung Kim @ 2023-05-24 17:41 ` Adrian Hunter 0 siblings, 0 replies; 7+ messages in thread From: Adrian Hunter @ 2023-05-24 17:41 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Andi Kleen, Masami Hiramatsu, Kan Liang On 24/05/23 19:10, Namhyung Kim wrote: > Hi Adrian, > > On Tue, May 23, 2023 at 11:11 PM Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 24/05/23 08:28, Namhyung Kim wrote: >>> Most of x86 instructions can have size suffix like b, w, l or q. >> >> (AT&T mnemonics) > > Right, will update. > >> >>> Instead of adding all these instructions in the table, we can handle >>> them in a general way. For example, it can try to find an instruction >>> as is. If not found, it'd try again without the suffix if it's one of >>> the allowed suffixes. >> >> I guess it might be possible that xyz is in the table but xyz<suffix> >> is a completely different instruction? > > Then xyz<suffix> should be in the table too. The match without > suffix is a fallback so it should find the correct instruction first. Right, so when adding xyz to the table, xyz<suffix> does not need to be added as well if it is the same instruction but with different operand sizes, but xyz<suffix> must be added as well if it is a different instruction with different ops. > > Thanks, > Namhyung > > >> >>> >>> Signed-off-by: Namhyung Kim <namhyung@kernel.org> >>> --- >>> tools/perf/util/annotate.c | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >>> index b708bbc49c9e..7f05f2a2aa83 100644 >>> --- a/tools/perf/util/annotate.c >>> +++ b/tools/perf/util/annotate.c >>> @@ -70,6 +70,7 @@ struct arch { >>> struct ins_ops *(*associate_instruction_ops)(struct arch *arch, const char *name); >>> bool sorted_instructions; >>> bool initialized; >>> + const char *insn_suffix; >>> void *priv; >>> unsigned int model; >>> unsigned int family; >>> @@ -179,6 +180,7 @@ static struct arch architectures[] = { >>> .init = x86__annotate_init, >>> .instructions = x86__instructions, >>> .nr_instructions = ARRAY_SIZE(x86__instructions), >>> + .insn_suffix = "bwlq", >>> .objdump = { >>> .comment_char = '#', >>> }, >>> @@ -720,6 +722,26 @@ static struct ins_ops *__ins__find(struct arch *arch, const char *name) >>> } >>> >>> ins = bsearch(name, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); >>> + if (ins) >>> + return ins->ops; >>> + >>> + if (arch->insn_suffix) { >>> + char tmp[32]; >>> + char suffix; >>> + size_t len = strlen(name); >>> + >>> + if (len == 0 || len >= sizeof(tmp)) >>> + return NULL; >>> + >>> + suffix = name[len - 1]; >>> + if (strchr(arch->insn_suffix, suffix) == NULL) >>> + return NULL; >>> + >>> + strcpy(tmp, name); >>> + tmp[len - 1] = '\0'; /* remove the suffix and check again */ >>> + >>> + ins = bsearch(tmp, arch->instructions, nmemb, sizeof(struct ins), ins__key_cmp); >>> + } >>> return ins ? ins->ops : NULL; >>> } >>> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-05-24 17:42 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-24 5:28 [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally Namhyung Kim 2023-05-24 5:28 ` [PATCH 2/2] perf annotate: Remove x86 instructions with suffix Namhyung Kim 2023-05-24 6:14 ` Adrian Hunter 2023-05-24 16:11 ` Namhyung Kim 2023-05-24 6:11 ` [PATCH 1/2] perf annotate: Handle x86 instruction suffix generally Adrian Hunter 2023-05-24 16:10 ` Namhyung Kim 2023-05-24 17:41 ` Adrian Hunter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).