This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] [ARC] Fix disassembler -M option.
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>
- Cc: binutils at sourceware dot org, Francois dot Bedard at synopsys dot com
- Date: Mon, 28 Nov 2016 11:54:55 +0000
- Subject: Re: [PATCH] [ARC] Fix disassembler -M option.
- Authentication-results: sourceware.org; auth=none
- References: <1480330894-29418-1-git-send-email-claziss@synopsys.com>
* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-11-28 12:01:34 +0100]:
> In ARC backend, we can use -M<feature> option to control how an opcode is disassembled.
> This patch fixes a number of issues introduced by the original commit:
> - fpus and fpud are swaped.
> - quarkse_em doesn't include FPX extensions.
> - auto guessed opcode mechanism may ignore options passed via -M<feature>.
>
> Ok to apply?
> Claudiu
Looks fine to me (I'm not a maintainer though) except for one minor
whitespace issue highlighted inline.
Thanks,
Andrew
>
> opcodes/
> 2016-11-28 Claudiu Zissulescu <claziss@synopsys.com>
>
> * arc-dis.c (is_compatible_p): Remove function.
> (skip_this_opcode): Don't add any decoding class to decode list.
> Remove warning.
> (find_format_from_table): Go through all opcodes, and warn if we
> use a guessed mnemonic.
>
> binutils/
> 2016-11-28 Claudiu Zissulescu <claziss@synopsys.com>
>
> * testsuite/binutils-all/arc/objdump.exp (Warning test): Update
> test.
> ---
> binutils/testsuite/binutils-all/arc/objdump.exp | 2 +-
> opcodes/arc-dis.c | 95 +++++++++++--------------
> 2 files changed, 41 insertions(+), 56 deletions(-)
>
> diff --git a/binutils/testsuite/binutils-all/arc/objdump.exp b/binutils/testsuite/binutils-all/arc/objdump.exp
> index 58c4a05..a988823 100644
> --- a/binutils/testsuite/binutils-all/arc/objdump.exp
> +++ b/binutils/testsuite/binutils-all/arc/objdump.exp
> @@ -46,7 +46,7 @@ if [is_remote host] {
>
> set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS --disassemble $objfile"]
>
> -set want "Warning: disassembly.*dsubh12\[ \t\]*r0,r2,r4.*dmulh12.f\[ \t\]*r0,r2,r4.*dmulh11.f"
> +set want "Warning: disassembly.*vmac2hnfr\[ \t\]*r0,r2,r4.*dmulh12.f\[ \t\]*r0,r2,r4.*dmulh11.f"
>
> if [regexp $want $got] then {
> pass "Warning test"
> diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
> index 31b5a91..beb12af 100644
> --- a/opcodes/arc-dis.c
> +++ b/opcodes/arc-dis.c
> @@ -108,21 +108,6 @@ static linkclass decodelist = NULL;
>
> /* Functions implementation. */
>
> -/* Return TRUE when two classes are not opcode conflicting. */
> -
> -static bfd_boolean
> -is_compatible_p (insn_class_t classA,
> - insn_subclass_t sclassA,
> - insn_class_t classB,
> - insn_subclass_t sclassB)
> -{
> - if (classA == DSP && sclassB == DPX)
> - return FALSE;
> - if (sclassA == DPX && classB == DSP)
> - return FALSE;
> - return TRUE;
> -}
> -
> /* Add a new element to the decode list. */
>
> static void
> @@ -141,54 +126,34 @@ add_to_decodelist (insn_class_t insn_class,
> disassembled. */
>
> static bfd_boolean
> -skip_this_opcode (const struct arc_opcode * opcode,
> - struct disassemble_info * info)
> +skip_this_opcode (const struct arc_opcode * opcode)
This should be 'const struct arc_opcode *opcode' I think. We might as
well fix it while the line is being edited.
> {
> linkclass t = decodelist;
> - bfd_boolean addme = TRUE;
>
> /* Check opcode for major 0x06, return if it is not in. */
> if (arc_opcode_len (opcode) == 4
> && OPCODE_32BIT_INSN (opcode->opcode) != 0x06)
> return FALSE;
>
> - while (t != NULL
> - && is_compatible_p (t->insn_class, t->subclass,
> - opcode->insn_class, opcode->subclass))
> + /* or not a known truble class. */
> + switch (opcode->insn_class)
> + {
> + case FLOAT:
> + case DSP:
> + break;
> + default:
> + return FALSE;
> + }
> +
> + while (t != NULL)
> {
> if ((t->insn_class == opcode->insn_class)
> && (t->subclass == opcode->subclass))
> - addme = FALSE;
> + return FALSE;
> t = t->nxt;
> }
>
> - /* If we found an incompatibility then we must skip. */
> - if (t != NULL)
> - return TRUE;
> -
> - /* Even if we do not precisely know the if the right mnemonics
> - is correctly displayed, keep the disassmbled code class
> - consistent. */
> - if (addme)
> - {
> - switch (opcode->insn_class)
> - {
> - case DSP:
> - case FLOAT:
> - /* Add to the conflict list only the classes which
> - counts. */
> - add_to_decodelist (opcode->insn_class, opcode->subclass);
> - /* Warn if we have to decode an opcode and no preferred
> - classes have been chosen. */
> - info->fprintf_func (info->stream, _("\n\
> -Warning: disassembly may be wrong due to guessed opcode class choice.\n\
> - Use -M<class[,class]> to select the correct opcode class(es).\n\t\t\t\t"));
> - break;
> - default:
> - break;
> - }
> - }
> - return FALSE;
> + return TRUE;
> }
>
> static bfd_vma
> @@ -243,8 +208,10 @@ find_format_from_table (struct disassemble_info *info,
> {
> unsigned int i = 0;
> const struct arc_opcode *opcode = NULL;
> + const struct arc_opcode *t_op = NULL;
> const unsigned char *opidx;
> const unsigned char *flgidx;
> + bfd_boolean warn_p = FALSE;
>
> do
> {
> @@ -337,15 +304,29 @@ find_format_from_table (struct disassemble_info *info,
> continue;
>
> if (insn_len == 4
> - && overlaps
> - && skip_this_opcode (opcode, info))
> - continue;
> + && overlaps)
> + {
> + warn_p = TRUE;
> + t_op = opcode;
> + if (skip_this_opcode (opcode))
> + continue;
> + }
>
> /* The instruction is valid. */
> return opcode;
> }
> while (opcode->mask);
>
> + if (warn_p)
> + {
> + info->fprintf_func (info->stream,
> + _("\nWarning: disassembly may be wrong due to "
> + "guessed opcode class choice.\n"
> + "Use -M<class[,class]> to select the correct "
> + "opcode class(es).\n\t\t\t\t"));
> + return t_op;
> + }
> +
> return NULL;
> }
>
> @@ -694,18 +675,22 @@ parse_option (char *option)
> add_to_decodelist (FLOAT, DPX);
>
> else if (CONST_STRNEQ (option, "quarkse_em"))
> - add_to_decodelist (FLOAT, QUARKSE);
> + {
> + add_to_decodelist (FLOAT, DPX);
> + add_to_decodelist (FLOAT, SPX);
> + add_to_decodelist (FLOAT, QUARKSE);
> + }
>
> else if (CONST_STRNEQ (option, "fpuda"))
> add_to_decodelist (FLOAT, DPA);
>
> - else if (CONST_STRNEQ (option, "fpud"))
> + else if (CONST_STRNEQ (option, "fpus"))
> {
> add_to_decodelist (FLOAT, SP);
> add_to_decodelist (FLOAT, CVT);
> }
>
> - else if (CONST_STRNEQ (option, "fpus"))
> + else if (CONST_STRNEQ (option, "fpud"))
> {
> add_to_decodelist (FLOAT, DP);
> add_to_decodelist (FLOAT, CVT);
> --
> 1.9.1
>