This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 13/20] MIPS/GAS: Improve instruction's mnemonic processing
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: Catherine Moore <clm at codesourcery dot com>, binutils at sourceware dot org
- Date: Fri, 10 Dec 2010 18:48:39 +0000 (GMT)
- Subject: Re: [PATCH 13/20] MIPS/GAS: Improve instruction's mnemonic processing
- References: <alpine.DEB.1.10.1012020129260.14129@tp.orcam.me.uk> <87oc8xalnq.fsf@firetop.home>
On Tue, 7 Dec 2010, Richard Sandiford wrote:
> > * config/tc-mips.c (mips_ip): Make a copy of the instruction's
> > mnemonic and use it for further processing.
>
> This patch doesn't preserve the current behaviour.
Ouch, I missed this subtlety, sorry. :(
> The current code treats the text after the "." as the first thing that
> should be matched against the format string. I assume it dates back to
> a time when certain types of operand suffix were handled using format
> characters. (Maybe the floating-point condition codes and formats?)
> This meant that things like:
>
> addu.$4,$5,$6
>
> were also acceptable, although of course:
>
> c.eq.d.$f0,$f2
>
> wasn't.
I did some archaeology and tracked down the introduction of this feature
to have happened between binutils 2.8 and 2.9. I checked the opcode table
differences between the two versions and found no modifications that would
justify the change. I looked at the relevant ChangeLog entries and there
is nothing about this change.
My only thought of any use for this stuff might be ITBL or some
unfinished work that has crept in by accident. The latter hypothesis is
further backed up by the lack of a corresponding ChangeLog entry. Either
way less than useful it would seem.
> The patch instead ignores everything after the ".", which means that
> we'd accept stuff like:
>
> addu.foobar $4,$5,$7
>
> (although again not "c.eq.d.foobar").
>
> I think we can simply remove the dot check. I don't see any testsuite
> regressions after doing that.
Yes, I agree. Here's a new version. No regressions for mips-sde-elf or
mips-linux-gnu.
2010-12-10 Maciej W. Rozycki <macro@codesourcery.com>
* config/tc-mips.c (mips_ip): Make a copy of the instruction's
mnemonic and use it for further processing.
OK?
Maciej
binutils-20101210-gas-mips-ip-insn-copy.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c 2010-12-10 18:17:43.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c 2010-12-10 18:17:43.000000000 +0000
@@ -8655,67 +8655,38 @@ mips_ip (char *str, struct mips_cl_insn
unsigned int lastpos = 0;
unsigned int limlo, limhi;
char *s_reset;
- char save_c = 0;
offsetT min_range, max_range;
+ char *name;
int argnum;
unsigned int rtype;
+ long end;
insn_error = NULL;
- /* If the instruction contains a '.', we first try to match an instruction
- including the '.'. Then we try again without the '.'. */
insn = NULL;
- for (s = str; *s != '\0' && !ISSPACE (*s); ++s)
- continue;
- /* If we stopped on whitespace, then replace the whitespace with null for
- the call to hash_find. Save the character we replaced just in case we
- have to re-parse the instruction. */
- if (ISSPACE (*s))
- {
- save_c = *s;
- *s++ = '\0';
- }
+ /* Try to match an instruction up to a space or to the end. */
+ for (end = 0; str[end] != '\0' && !ISSPACE (str[end]); end++)
+ continue;
- insn = (struct mips_opcode *) hash_find (op_hash, str);
+ /* Make a copy of the instruction so that we can fiddle with it. */
+ name = alloca (end + 1);
+ memcpy (name, str, end);
+ name[end] = '\0';
- /* If we didn't find the instruction in the opcode table, try again, but
- this time with just the instruction up to, but not including the
- first '.'. */
+ insn = (struct mips_opcode *) hash_find (op_hash, name);
if (insn == NULL)
{
- /* Restore the character we overwrite above (if any). */
- if (save_c)
- *(--s) = save_c;
-
- /* Scan up to the first '.' or whitespace. */
- for (s = str;
- *s != '\0' && *s != '.' && !ISSPACE (*s);
- ++s)
- continue;
-
- /* If we did not find a '.', then we can quit now. */
- if (*s != '.')
- {
- insn_error = _("Unrecognized opcode");
- return;
- }
-
- /* Lookup the instruction in the hash table. */
- *s++ = '\0';
- if ((insn = (struct mips_opcode *) hash_find (op_hash, str)) == NULL)
- {
- insn_error = _("Unrecognized opcode");
- return;
- }
+ insn_error = _("Unrecognized opcode");
+ return;
}
- argsStart = s;
+ argsStart = s = str + end;
for (;;)
{
bfd_boolean ok;
- gas_assert (strcmp (insn->name, str) == 0);
+ gas_assert (strcmp (insn->name, name) == 0);
ok = is_opcode_valid (insn);
if (! ok)
@@ -8737,8 +8708,6 @@ mips_ip (char *str, struct mips_cl_insn
mips_cpu_info_from_isa (mips_opts.isa)->name);
insn_error = buf;
}
- if (save_c)
- *(--s) = save_c;
return;
}
}
@@ -10121,8 +10090,6 @@ mips_ip (char *str, struct mips_cl_insn
insn_error = _("Illegal operands");
continue;
}
- if (save_c)
- *(--argsStart) = save_c;
insn_error = _("Illegal operands");
return;
}