Re: [PATCH v5 2/7] perf annotate: Add cross arch annotate support

From: Russell King - ARM Linux
Date: Fri Aug 19 2016 - 03:50:54 EST


On Fri, Aug 19, 2016 at 10:59:01AM +0530, Ravi Bangoria wrote:
> -static struct ins instructions[] = {
> +static struct ins instructions_x86[] = {
> { .name = "add", .ops = &mov_ops, },
> { .name = "addl", .ops = &mov_ops, },
> { .name = "addq", .ops = &mov_ops, },
> { .name = "addw", .ops = &mov_ops, },
> { .name = "and", .ops = &mov_ops, },
> -#ifdef __arm__
> - { .name = "b", .ops = &jump_ops, }, // might also be a call
> - { .name = "bcc", .ops = &jump_ops, },
> - { .name = "bcs", .ops = &jump_ops, },
> - { .name = "beq", .ops = &jump_ops, },
> - { .name = "bge", .ops = &jump_ops, },
> - { .name = "bgt", .ops = &jump_ops, },
> - { .name = "bhi", .ops = &jump_ops, },
> - { .name = "bl", .ops = &call_ops, },
> - { .name = "bls", .ops = &jump_ops, },
> - { .name = "blt", .ops = &jump_ops, },
> - { .name = "blx", .ops = &call_ops, },
> - { .name = "bne", .ops = &jump_ops, },
> -#endif

Notice that ARM includes a lot of other instructions from this table,
not just those above.

> { .name = "bts", .ops = &mov_ops, },
> { .name = "call", .ops = &call_ops, },
> { .name = "callq", .ops = &call_ops, },
> @@ -456,6 +444,21 @@ static struct ins instructions[] = {
> { .name = "retq", .ops = &ret_ops, },
> };
>
> +static struct ins instructions_arm[] = {
> + { .name = "b", .ops = &jump_ops, }, /* might also be a call */
> + { .name = "bcc", .ops = &jump_ops, },
> + { .name = "bcs", .ops = &jump_ops, },
> + { .name = "beq", .ops = &jump_ops, },
> + { .name = "bge", .ops = &jump_ops, },
> + { .name = "bgt", .ops = &jump_ops, },
> + { .name = "bhi", .ops = &jump_ops, },
> + { .name = "bl", .ops = &call_ops, },
> + { .name = "bls", .ops = &jump_ops, },
> + { .name = "blt", .ops = &jump_ops, },
> + { .name = "blx", .ops = &call_ops, },
> + { .name = "bne", .ops = &jump_ops, },
> +};
> +
...
> + if (!strcmp(norm_arch, NORM_X86)) {
> + instructions = instructions_x86;
> + nmemb = ARRAY_SIZE(instructions_x86);
> + } else if (!strcmp(norm_arch, NORM_ARM)) {
> + instructions = instructions_arm;
> + nmemb = ARRAY_SIZE(instructions_arm);

But these changes result in _only_ the ones that were in the #if __arm__
being matched. This is wrong.

If we want to go that way, we need to add _all_ arm instructions to
instructions_arm, not just those within the #if.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.