This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
- From: Nick Clifton <nickc at redhat dot com>
- To: Andrew Burgess <andrew dot burgess at embecosm dot com>, binutils at sourceware dot org
- Cc: Claudiu dot Zissulescu at synopsys dot com, Cupertino dot Miranda at synopsys dot com, noamca at mellanox dot com
- Date: Wed, 13 Apr 2016 14:32:38 +0100
- Subject: Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1460458691 dot git dot andrew dot burgess at embecosm dot com> <cb64992909da171def628e036ee5f6cc94e71058 dot 1460458691 dot git dot andrew dot burgess at embecosm dot com>
Hi Andrew,
> opcodes/ChangeLog:
>
> * arc-dis.c (arc_insn_length): New function.
> (print_insn_arc): Use arc_insn_length.
Approved - please apply, but ...
> +static int
> +arc_insn_length (bfd_byte msb, bfd_byte lsb ATTRIBUTE_UNUSED,
> + struct disassemble_info *info)
Would this function ever return a negative value ? I assume not, so
it would make sense for its return type to be "unsigned int".
Also will the LSB parameter ever be used ? I assume that it is there
for a future extension to this function which might need it, but if
that is only theoretical, then maybe the parameter can be dropped
entirely and the code simplified.
> + switch (info->mach)
> + {
> + case bfd_mach_arc_nps400:
> + case bfd_mach_arc_arc700:
> + case bfd_mach_arc_arc600:
> + len = (major_opcode > 0xb) ? 2 : 4;
> + break;
> +
> + case bfd_mach_arc_arcv2:
> + len = (major_opcode > 0x7) ? 2 : 4;
> + break;
> +
> + default:
> + abort ();
> + }
> +
> + return len;
Since len is always returned without any further processing you
could save a bit of space in the function by moving the return
instruction into the switch statement. eg:
case bfd_mach_arc_arc600:
return (major_opcode > 0xb) ? 2 : 4;
Cheers
Nick