Re: [PATCH 13/20] MIPS/GAS: Improve instruction's mnemonic processing

["Maciej W. Rozycki" <macro at codesourcery dot com>]

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;
     }