[PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[Andrew Burgess]

The assembler builds a hash to hold references to arc_opcode entries in
the arc_opcodes table.  This hash assumes that each mnemonic will always
appear in contiguous blocks within the arc_opcodes table, so all ADD
instruction will be together, all AND instructions will likewise be
together and so on.

The problem with this is that as different variations of arc are added,
then it is often more convenient to split instructions apart, so all the
base ADD instructions are together, but, variants of ADD specific to one
variation of arc are grouped with other instructions specific to that
arc variant.  The current data structures don't support this
configuration of instructions.

This commit is a first step towards addressing this problem.  In this
commit the hash table that currently holds arc_opcode pointers directly,
instead holds a pointer to a new, intermediate, data structure.  This
new data structure holds the pointer to the arc_opcode.  In this way, we
can, in a future commit support having the intermediate structure hold
multiple pointers to different arc_opcode groups.

gas/ChangeLog:

	* config/tc-arc.c (struct arc_opcode_hash_entry): New structure.
	(arc_find_opcode): New function.
	(find_special_case_pseudo): Use arc_find_opcode.
	(find_special_case_flag): Likewise.
	(assemble_tokens): Likewise.
	(md_begin): Build hash using struct arc_opcode_hash_entry.
---
 gas/ChangeLog       |  9 ++++++++
 gas/config/tc-arc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 4f9c336..3c093ca 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -305,6 +305,16 @@ static struct arc_last_insn
   bfd_boolean has_delay_slot;
 } arc_last_insns[2];
 
+/* Structure to hold an entry in ARC_OPCODE_HASH.  */
+struct arc_opcode_hash_entry
+{
+  /* The number of pointers in the OPCODE list.  */
+  size_t count;
+
+  /* Points to a list of opcode pointers.  */
+  const struct arc_opcode **opcode;
+};
+
 /* Forward declaration.  */
 static void assemble_insn
   (const struct arc_opcode *, const expressionS *, int,
@@ -551,6 +561,29 @@ static bfd_boolean assembling_insn = FALSE;
 
 /* Functions implementation.  */
 
+/* Return a pointer to the first entry in ARC_OPCODE_HASH that matches
+   NAME, or NULL if there are no matching entries.  */
+
+static const struct arc_opcode *
+arc_find_opcode (const char *name)
+{
+  const struct arc_opcode_hash_entry *entry;
+  const struct arc_opcode *opcode;
+
+  entry = hash_find (arc_opcode_hash, name);
+  if (entry != NULL)
+    {
+      if (entry->count > 1)
+        as_fatal (_("unable to lookup `%s', too many opcode chains"),
+                  name);
+      opcode = *entry->opcode;
+    }
+  else
+    opcode = NULL;
+
+  return opcode;
+}
+
 /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
    is encoded as 'middle-endian' for a little-endian target.  FIXME!
    this function is used for regular 4 byte instructions as well.  */
@@ -1871,8 +1904,7 @@ find_special_case_pseudo (const char *opname,
       break;
     }
 
-  return (const struct arc_opcode *)
-    hash_find (arc_opcode_hash,	pseudo_insn->mnemonic_r);
+  return arc_find_opcode (pseudo_insn->mnemonic_r);
 }
 
 static const struct arc_opcode *
@@ -1908,9 +1940,7 @@ find_special_case_flag (const char *opname,
 	  flaglen = strlen (flagnm);
 	  if (strcmp (opname + oplen, flagnm) == 0)
 	    {
-	      opcode = (const struct arc_opcode *)
-		hash_find (arc_opcode_hash,
-			   arc_flag_special_opcode->name);
+              opcode = arc_find_opcode (arc_flag_special_opcode->name);
 
 	      if (*nflgs + 1 > MAX_INSN_FLGS)
 		break;
@@ -2000,7 +2030,7 @@ assemble_tokens (const char *opname,
   int cpumatch = 1;
 
   /* Search opcodes.  */
-  opcode = (const struct arc_opcode *) hash_find (arc_opcode_hash, opname);
+  opcode = arc_find_opcode (opname);
 
   /* Couldn't find opcode conventional way, try special cases.  */
   if (!opcode)
@@ -2147,12 +2177,28 @@ md_begin (void)
   for (i = 0; i < arc_num_opcodes;)
     {
       const char *name, *retval;
+      struct arc_opcode_hash_entry *entry;
 
       name = arc_opcodes[i].name;
-      retval = hash_insert (arc_opcode_hash, name, (void *) &arc_opcodes[i]);
-      if (retval)
-	as_fatal (_("internal error: can't hash opcode '%s': %s"),
-		  name, retval);
+
+      entry = hash_find (arc_opcode_hash, name);
+      if (entry == NULL)
+        {
+          entry = xmalloc (sizeof (*entry));
+          entry->count = 0;
+          entry->opcode = NULL;
+
+          retval = hash_insert (arc_opcode_hash, name, (void *) entry);
+          if (retval)
+            as_fatal (_("internal error: can't hash opcode '%s': %s"),
+                      name, retval);
+        }
+
+      entry->opcode = xrealloc (entry->opcode,
+                                sizeof (const struct arc_opcode *)
+                                * entry->count + 1);
+      entry->opcode [entry->count] = &arc_opcodes[i];
+      entry->count++;
 
       while (++i < arc_num_opcodes
 	     && (arc_opcodes[i].name == name
-- 
2.5.1

[PATCH 4/6] gas/arc: Handle multiple arc_opcode chains for same mnemonic

[Andrew Burgess]

This commit completes support for having multiple instructions with the
same mnemonic in non-contiguous blocks within the arc_opcodes table.
The commit adds an iterator mechanism for the arc_opcode_hash_entry
structure, which is then used in find_opcode_match to consider all
arc_opcode entries with the same mnemonic.

gas/ChangeLog:

	* config/tc-arc.c (struct arc_opcode_hash_entry_iterator): New
	structure.
	(arc_opcode_hash_entry_iterator_init): New function.
	(arc_opcode_hash_entry_iterator_next): New function.
	(find_opcode_match): Iterate over all arc_opcode entries
	referenced by the arc_opcode_hash_entry passed in as a parameter.
---
 gas/ChangeLog       |  9 +++++++
 gas/config/tc-arc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index f80bd2d..45bf30f 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -315,6 +315,17 @@ struct arc_opcode_hash_entry
   const struct arc_opcode **opcode;
 };
 
+/* Structure used for iterating through an arc_opcode_hash_entry.  */
+struct arc_opcode_hash_entry_iterator
+{
+  /* Index into the OPCODE element of the arc_opcode_hash_entry.  */
+  size_t index;
+
+  /* The specific ARC_OPCODE from the ARC_OPCODES table that was last
+     returned by this iterator.  */
+  const struct arc_opcode *opcode;
+};
+
 /* Forward declaration.  */
 static void assemble_insn
   (const struct arc_opcode *, const expressionS *, int,
@@ -574,6 +585,47 @@ arc_find_opcode (const char *name)
   return entry;
 }
 
+/* Initialise the iterator ITER.  */
+
+static void
+arc_opcode_hash_entry_iterator_init (struct arc_opcode_hash_entry_iterator *iter)
+{
+  iter->index = 0;
+  iter->opcode = NULL;
+}
+
+/* Return the next ARC_OPCODE from ENTRY, using ITER to hold state between
+   calls to this function.  Return NULL when all ARC_OPCODE entries have
+   been returned.  */
+
+static const struct arc_opcode *
+arc_opcode_hash_entry_iterator_next (const struct arc_opcode_hash_entry *entry,
+				     struct arc_opcode_hash_entry_iterator *iter)
+{
+  if (iter->opcode == NULL && iter->index == 0)
+    {
+      gas_assert (entry->count > 0);
+      iter->opcode = entry->opcode[iter->index];
+    }
+  else if (iter->opcode != NULL)
+    {
+      const char *old_name = iter->opcode->name;
+
+      iter->opcode++;
+      if ((iter->opcode - arc_opcodes >= (int) arc_num_opcodes)
+	  || (strcmp (old_name, iter->opcode->name) != 0))
+	{
+	  iter->index++;
+	  if (iter->index == entry->count)
+	    iter->opcode = NULL;
+	  else
+	    iter->opcode = entry->opcode[iter->index];
+	}
+    }
+
+  return iter->opcode;
+}
+
 /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
    is encoded as 'middle-endian' for a little-endian target.  FIXME!
    this function is used for regular 4 byte instructions as well.  */
@@ -1378,23 +1430,22 @@ find_opcode_match (const struct arc_opcode_hash_entry *entry,
 		   int nflgs,
 		   int *pcpumatch)
 {
-  const struct arc_opcode *first_opcode = entry->opcode[0];
-  const struct arc_opcode *opcode = first_opcode;
+  const struct arc_opcode *opcode;
+  struct arc_opcode_hash_entry_iterator iter;
   int ntok = *pntok;
   int got_cpu_match = 0;
   expressionS bktok[MAX_INSN_ARGS];
   int bkntok;
   expressionS emptyE;
 
-  gas_assert (entry->count > 0);
-  if (entry->count > 1)
-    as_fatal (_("unable to lookup `%s', too many opcode chains"),
-	      first_opcode->name);
+  arc_opcode_hash_entry_iterator_init (&iter);
   memset (&emptyE, 0, sizeof (emptyE));
   memcpy (bktok, tok, MAX_INSN_ARGS * sizeof (*tok));
   bkntok = ntok;
 
-  do
+  for (opcode = arc_opcode_hash_entry_iterator_next (entry, &iter);
+       opcode != NULL;
+       opcode = arc_opcode_hash_entry_iterator_next (entry, &iter))
     {
       const unsigned char *opidx;
       const unsigned char *flgidx;
@@ -1713,8 +1764,6 @@ find_opcode_match (const struct arc_opcode_hash_entry *entry,
       memcpy (tok, bktok, MAX_INSN_ARGS * sizeof (*tok));
       ntok = bkntok;
     }
-  while (++opcode - arc_opcodes < (int) arc_num_opcodes
-	 && !strcmp (opcode->name, first_opcode->name));
 
   if (*pcpumatch)
     *pcpumatch = got_cpu_match;
@@ -2024,9 +2073,8 @@ assemble_tokens (const char *opname,
     {
       const struct arc_opcode *opcode;
 
-      pr_debug ("%s:%d: assemble_tokens: %s trying opcode 0x%08X\n",
-		frag_now->fr_file, frag_now->fr_line, opcode->name,
-		opcode->opcode);
+      pr_debug ("%s:%d: assemble_tokens: %s\n",
+		frag_now->fr_file, frag_now->fr_line, opname);
       found_something = TRUE;
       opcode = find_opcode_match (entry, tok, &ntok, pflags,
 				  nflgs, &cpumatch);
-- 
2.5.1

[PATCH 2/6] gas/arc: Remove preprocess_operands function

[Andrew Burgess]

The preprocess_operands function changes the incoming list of assembler
tokens based on the assumption that the first arc_operand found will be
the same instruction class as all of the arc_operands for the same
mnemonic.

Though this assumption is probably fine, removing this assumption, and
pushing the token change down into assemble_tokens makes the code no
more complex, and might even be easier to follow.

gas/ChangeLog:

	* config/tc-arc.c (find_opcode_match): Handle O_symbol case, add
	new de_fault label.
	(preprocess_operands): Delete.
	(assemble_tokens): Remove call to preprocess_operands.
---
 gas/ChangeLog       |  7 +++++
 gas/config/tc-arc.c | 75 ++++++++++++++++++++++-------------------------------
 2 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 3c093ca..814691c 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -1512,6 +1512,36 @@ find_opcode_match (const struct arc_opcode *first_opcode,
 		  ++ntok;
 		  break;
 
+		case O_symbol:
+		  {
+		    const char *p;
+		    size_t len;
+		    const struct arc_aux_reg *auxr;
+		    unsigned j;
+
+		    if (opcode->class != AUXREG)
+		      goto de_fault;
+		    p = S_GET_NAME (tok[tokidx].X_add_symbol);
+		    len = strlen (p);
+		    auxr = &arc_aux_regs[0];
+
+		    for (j = 0; j < arc_num_aux_regs; j++, auxr++)
+		      if (len == auxr->length
+			  && strcasecmp (auxr->name, p) == 0)
+			{
+			  /* We modify the token array here, safe in the
+			     knowledge, that if this was the wrong choice
+			     then the original contents will be restored
+			     from BKTOK.  */
+			  tok[tokidx].X_op = O_constant;
+			  tok[tokidx].X_add_number = auxr->address;
+			  break;
+			}
+
+		    if (tok[tokidx].X_op != O_constant)
+		      goto de_fault;
+		  }
+		  /* Fall-through */
 		case O_constant:
 		  /* Check the range.  */
 		  if (operand->bits != 32
@@ -1584,6 +1614,7 @@ find_opcode_match (const struct arc_opcode *first_opcode,
 		      break;
 		    }
 		default:
+		de_fault:
 		  if (operand->default_reloc == 0)
 		    goto match_failed; /* The operand needs relocation.  */
 
@@ -1973,48 +2004,6 @@ find_special_case (const char *opname,
   return opcode;
 }
 
-static void
-preprocess_operands (const struct arc_opcode *opcode,
-		     expressionS *tok,
-		     int ntok)
-{
-  int i;
-  size_t len;
-  const char *p;
-  unsigned j;
-  const struct arc_aux_reg *auxr;
-
-  for (i = 0; i < ntok; i++)
-    {
-      switch (tok[i].X_op)
-	{
-	case O_illegal:
-	case O_absent:
-	  break; /* Throw and error.  */
-
-	case O_symbol:
-	  if (opcode->class != AUXREG)
-	    break;
-	  /* Convert the symbol to a constant if possible.  */
-	  p = S_GET_NAME (tok[i].X_add_symbol);
-	  len = strlen (p);
-
-	  auxr = &arc_aux_regs[0];
-	  for (j = 0; j < arc_num_aux_regs; j++, auxr++)
-	    if (len == auxr->length
-		&& strcasecmp (auxr->name, p) == 0)
-	      {
-		tok[i].X_op = O_constant;
-		tok[i].X_add_number = auxr->address;
-		break;
-	      }
-	  break;
-	default:
-	  break;
-	}
-    }
-}
-
 /* Given an opcode name, pre-tockenized set of argumenst and the
    opcode flags, take it all the way through emission.  */
 
@@ -2042,8 +2031,6 @@ assemble_tokens (const char *opname,
 		frag_now->fr_file, frag_now->fr_line, opcode->name,
 		opcode->opcode);
 
-      preprocess_operands (opcode, tok, ntok);
-
       found_something = TRUE;
       opcode = find_opcode_match (opcode, tok, &ntok, pflags, nflgs, &cpumatch);
       if (opcode)
-- 
2.5.1

Re: [PATCH 2/6] gas/arc: Remove preprocess_operands function

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (find_opcode_match): Handle O_symbol case, add
> 	new de_fault label.
> 	(preprocess_operands): Delete.
> 	(assemble_tokens): Remove call to preprocess_operands.
 
Approved - please apply.

Cheers
  Nick

[PATCH 5/6] gas/arc: Allow greater range of characters into flag names

[Andrew Burgess]

There's little point limiting the characters that can be accepted into
flag names.  It does mean earlier error detection, but invalid flags
will be detected later in the process anyway, and limiting the
acceptable character range earlier only means that when a new flag is
added that uses a previously unused character the programmer must
remember to update this part of the assembler.

For now all alpha-numeric characters seems like a reasonable set to
allow.

gas/ChangeLog:

	* config/tc-arc.c (tokenize_flags): Allow greater range of
	characters into flag names.
---
 gas/ChangeLog       | 5 +++++
 gas/config/tc-arc.c | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 45bf30f..4fd5403 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -1152,7 +1152,8 @@ tokenize_flags (const char *str,
 	  if (num_flags >= nflg)
 	    goto err;
 
-	  flgnamelen = strspn (input_line_pointer, "abcdefghilmnopqrstvwxz");
+	  flgnamelen = strspn (input_line_pointer,
+			       "abcdefghijklmnopqrstuvwxyz0123456789");
 	  if (flgnamelen > MAX_FLAG_NAME_LENGTH)
 	    goto err;
 
-- 
2.5.1

RE: [PATCH 5/6] gas/arc: Allow greater range of characters into flag names

[Claudiu Zissulescu]

Ok.
Cheers,
Claudiu

> -----Original Message-----
> From: Andrew Burgess [mailto:andrew.burgess@embecosm.com]
> Sent: Sunday, April 03, 2016 12:51 AM
> To: binutils@sourceware.org
> Cc: Claudiu.Zissulescu@synopsys.com; noamca@mellanox.com; Andrew
> Burgess
> Subject: [PATCH 5/6] gas/arc: Allow greater range of characters into flag
> names
> 
> There's little point limiting the characters that can be accepted into
> flag names.  It does mean earlier error detection, but invalid flags
> will be detected later in the process anyway, and limiting the
> acceptable character range earlier only means that when a new flag is
> added that uses a previously unused character the programmer must
> remember to update this part of the assembler.
> 
> For now all alpha-numeric characters seems like a reasonable set to
> allow.
> 
> gas/ChangeLog:
> 
> 	* config/tc-arc.c (tokenize_flags): Allow greater range of
> 	characters into flag names.
> ---
>  gas/ChangeLog       | 5 +++++
>  gas/config/tc-arc.c | 3 ++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
> index 45bf30f..4fd5403 100644
> --- a/gas/config/tc-arc.c
> +++ b/gas/config/tc-arc.c
> @@ -1152,7 +1152,8 @@ tokenize_flags (const char *str,
>  	  if (num_flags >= nflg)
>  	    goto err;
> 
> -	  flgnamelen = strspn (input_line_pointer,
> "abcdefghilmnopqrstvwxz");
> +	  flgnamelen = strspn (input_line_pointer,
> +			       "abcdefghijklmnopqrstuvwxyz0123456789");
>  	  if (flgnamelen > MAX_FLAG_NAME_LENGTH)
>  	    goto err;
> 
> --
> 2.5.1

Re: [PATCH 5/6] gas/arc: Allow greater range of characters into flag names

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (tokenize_flags): Allow greater range of
> 	characters into flag names.

Approved - please apply.

Cheers
  Nick

[PATCH 3/6] gas/arc: Additional work to support multiple arc_opcode chains

[Andrew Burgess]

Building on earlier commits, this commit pushes along support for having
multiple arc_opcode entries in the arc_opcodes table that have the same
mnemonic (name) field, but are not stored in a contiguous block in the
table.

In this commit we support looking up the arc_opcode_hash_entry from the
has table, and passing this along to the find_opcode_match function,
which finds the specific arc_opcode that we're assembling.  We still
don't actually support the multiple chains of arc_opcode entries in this
commit, but the problem is now isolated to find_opcode_match.

There is no functional change in this commit over the current state of
the assembler.

gas/ChangeLog:

	* config/tc-arc.c (arc_find_opcode): Now returns
	arc_opcode_hash_entry pointer.
	(find_opcode_match): Update argument type, extract arc_opcode from
	incoming arc_opcode_hash_entry.
	(find_special_case_pseudo): Update return type.
	(find_special_case_flag): Update return type.
	(find_special_case): Update return type.
	(assemble_tokens): Lookup arc_opcode_hash_entry based on
	instruction mnemonic, then use find_opcode_match to identify
	specific arc_opcode.
---
 gas/ChangeLog       | 13 ++++++++++
 gas/config/tc-arc.c | 68 ++++++++++++++++++++++++++---------------------------
 2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 814691c..f80bd2d 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -561,27 +561,17 @@ static bfd_boolean assembling_insn = FALSE;
 
 /* Functions implementation.  */
 
-/* Return a pointer to the first entry in ARC_OPCODE_HASH that matches
-   NAME, or NULL if there are no matching entries.  */
+/* Return a pointer to ARC_OPCODE_HASH_ENTRY that identifies all
+   ARC_OPCODE entries in ARC_OPCODE_HASH that match NAME, or NULL if there
+   are no matching entries in ARC_OPCODE_HASH.  */
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 arc_find_opcode (const char *name)
 {
   const struct arc_opcode_hash_entry *entry;
-  const struct arc_opcode *opcode;
 
   entry = hash_find (arc_opcode_hash, name);
-  if (entry != NULL)
-    {
-      if (entry->count > 1)
-        as_fatal (_("unable to lookup `%s', too many opcode chains"),
-                  name);
-      opcode = *entry->opcode;
-    }
-  else
-    opcode = NULL;
-
-  return opcode;
+  return entry;
 }
 
 /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
@@ -1381,13 +1371,14 @@ allocate_tok (expressionS *tok, int ntok, int cidx)
    syntax match.  */
 
 static const struct arc_opcode *
-find_opcode_match (const struct arc_opcode *first_opcode,
+find_opcode_match (const struct arc_opcode_hash_entry *entry,
 		   expressionS *tok,
 		   int *pntok,
 		   struct arc_flags *first_pflag,
 		   int nflgs,
 		   int *pcpumatch)
 {
+  const struct arc_opcode *first_opcode = entry->opcode[0];
   const struct arc_opcode *opcode = first_opcode;
   int ntok = *pntok;
   int got_cpu_match = 0;
@@ -1395,6 +1386,10 @@ find_opcode_match (const struct arc_opcode *first_opcode,
   int bkntok;
   expressionS emptyE;
 
+  gas_assert (entry->count > 0);
+  if (entry->count > 1)
+    as_fatal (_("unable to lookup `%s', too many opcode chains"),
+	      first_opcode->name);
   memset (&emptyE, 0, sizeof (emptyE));
   memcpy (bktok, tok, MAX_INSN_ARGS * sizeof (*tok));
   bkntok = ntok;
@@ -1843,7 +1838,7 @@ find_pseudo_insn (const char *opname,
 
 /* Assumes the expressionS *tok is of sufficient size.  */
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 find_special_case_pseudo (const char *opname,
 			  int *ntok,
 			  expressionS *tok,
@@ -1938,7 +1933,7 @@ find_special_case_pseudo (const char *opname,
   return arc_find_opcode (pseudo_insn->mnemonic_r);
 }
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 find_special_case_flag (const char *opname,
 			int *nflgs,
 			struct arc_flags *pflags)
@@ -1948,7 +1943,7 @@ find_special_case_flag (const char *opname,
   unsigned flag_idx, flag_arr_idx;
   size_t flaglen, oplen;
   const struct arc_flag_special *arc_flag_special_opcode;
-  const struct arc_opcode *opcode;
+  const struct arc_opcode_hash_entry *entry;
 
   /* Search for special case instruction.  */
   for (i = 0; i < arc_num_flag_special; i++)
@@ -1971,14 +1966,14 @@ find_special_case_flag (const char *opname,
 	  flaglen = strlen (flagnm);
 	  if (strcmp (opname + oplen, flagnm) == 0)
 	    {
-              opcode = arc_find_opcode (arc_flag_special_opcode->name);
+              entry = arc_find_opcode (arc_flag_special_opcode->name);
 
 	      if (*nflgs + 1 > MAX_INSN_FLGS)
 		break;
 	      memcpy (pflags[*nflgs].name, flagnm, flaglen);
 	      pflags[*nflgs].name[flaglen] = '\0';
 	      (*nflgs)++;
-	      return opcode;
+	      return entry;
 	    }
 	}
     }
@@ -1987,21 +1982,21 @@ find_special_case_flag (const char *opname,
 
 /* Used to find special case opcode.  */
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 find_special_case (const char *opname,
 		   int *nflgs,
 		   struct arc_flags *pflags,
 		   expressionS *tok,
 		   int *ntok)
 {
-  const struct arc_opcode *opcode;
+  const struct arc_opcode_hash_entry *entry;
 
-  opcode = find_special_case_pseudo (opname, ntok, tok, nflgs, pflags);
+  entry = find_special_case_pseudo (opname, ntok, tok, nflgs, pflags);
 
-  if (opcode == NULL)
-    opcode = find_special_case_flag (opname, nflgs, pflags);
+  if (entry == NULL)
+    entry = find_special_case_flag (opname, nflgs, pflags);
 
-  return opcode;
+  return entry;
 }
 
 /* Given an opcode name, pre-tockenized set of argumenst and the
@@ -2015,27 +2010,30 @@ assemble_tokens (const char *opname,
 		 int nflgs)
 {
   bfd_boolean found_something = FALSE;
-  const struct arc_opcode *opcode;
+  const struct arc_opcode_hash_entry *entry;
   int cpumatch = 1;
 
   /* Search opcodes.  */
-  opcode = arc_find_opcode (opname);
+  entry = arc_find_opcode (opname);
 
   /* Couldn't find opcode conventional way, try special cases.  */
-  if (!opcode)
-    opcode = find_special_case (opname, &nflgs, pflags, tok, &ntok);
+  if (entry == NULL)
+    entry = find_special_case (opname, &nflgs, pflags, tok, &ntok);
 
-  if (opcode)
+  if (entry != NULL)
     {
+      const struct arc_opcode *opcode;
+
       pr_debug ("%s:%d: assemble_tokens: %s trying opcode 0x%08X\n",
 		frag_now->fr_file, frag_now->fr_line, opcode->name,
 		opcode->opcode);
-
       found_something = TRUE;
-      opcode = find_opcode_match (opcode, tok, &ntok, pflags, nflgs, &cpumatch);
-      if (opcode)
+      opcode = find_opcode_match (entry, tok, &ntok, pflags,
+				  nflgs, &cpumatch);
+      if (opcode != NULL)
 	{
 	  struct arc_insn insn;
+
 	  assemble_insn (opcode, tok, ntok, pflags, nflgs, &insn);
 	  emit_insn (&insn);
 	  return;
-- 
2.5.1

[PATCH 6/6] arc/nps400: Add new control instructions

[Andrew Burgess]

Add some new control instructions to the opcodes library, and a new test
for these new instructions to the assembler.  The new instructions use
an instruction flag longer than any seen before (on arc), and so the max
flag length is extended to accommodate this.

gas/ChangeLog:

	* config/tc-arc.h (MAX_FLAG_NAME_LENGTH): Increase to 7.
	* testsuite/gas/arc/nps400-2.d: New file.
	* testsuite/gas/arc/nps400-2.s: New file.

opcodes/ChangeLog:

	* arc-nps400-tbl.h: Add schd, sync, and hwschd instructions.
	* arc-opc.c (arc_flag_operands): Add new flags.
	(arc_flag_classes): Add new classes.
---
 gas/ChangeLog                    |  6 +++++
 gas/config/tc-arc.h              |  2 +-
 gas/testsuite/gas/arc/nps400-2.d | 18 +++++++++++++++
 gas/testsuite/gas/arc/nps400-2.s | 16 +++++++++++++
 opcodes/ChangeLog                |  6 +++++
 opcodes/arc-nps400-tbl.h         | 17 ++++++++++++++
 opcodes/arc-opc.c                | 50 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 114 insertions(+), 1 deletion(-)
 create mode 100644 gas/testsuite/gas/arc/nps400-2.d
 create mode 100644 gas/testsuite/gas/arc/nps400-2.s

diff --git a/gas/config/tc-arc.h b/gas/config/tc-arc.h
index 93a7d77..7bd281c 100644
--- a/gas/config/tc-arc.h
+++ b/gas/config/tc-arc.h
@@ -210,7 +210,7 @@ extern int tc_arc_regname_to_dw2regnum (char *regname);
 #define NOP_OPCODE_S   0x000078E0
 #define NOP_OPCODE_L   0x264A7000 /* mov 0,0.  */
 
-#define MAX_FLAG_NAME_LENGTH 3
+#define MAX_FLAG_NAME_LENGTH 7
 
 struct arc_flags
 {
diff --git a/gas/testsuite/gas/arc/nps400-2.d b/gas/testsuite/gas/arc/nps400-2.d
new file mode 100644
index 0000000..966907e
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-2.d
@@ -0,0 +1,18 @@
+#as: -mcpu=nps400
+#objdump: -dr
+
+.*: +file format .*arc.*
+
+Disassembly of section .text:
+
+[0-9a-f]+ <.*>:
+   0:	3e6f 7004           	schd\.rw
+   4:	3e6f 7084           	schd\.rd
+   8:	3e6f 7044           	schd\.wft
+   c:	3e6f 7144           	schd\.wft\.ie1
+  10:	3e6f 7244           	schd\.wft\.ie2
+  14:	3e6f 7344           	schd\.wft\.ie12
+  18:	3e6f 703f           	sync\.rd
+  1c:	3e6f 707f           	sync\.wr
+  20:	3a6f 10bf           	hwschd\.off	r10
+  24:	3e6f 7503           	hwschd\.restore	0,r20
diff --git a/gas/testsuite/gas/arc/nps400-2.s b/gas/testsuite/gas/arc/nps400-2.s
new file mode 100644
index 0000000..de4c839
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-2.s
@@ -0,0 +1,16 @@
+        .text
+        ;; schd
+        schd.rw
+        schd.rd
+        schd.wft
+        schd.wft.ie1
+        schd.wft.ie2
+        schd.wft.ie12
+
+        ;; sync
+        sync.rd
+        sync.wr
+
+        ;; hwschd
+        hwschd.off      r10
+        hwschd.restore  0, r20
diff --git a/opcodes/arc-nps400-tbl.h b/opcodes/arc-nps400-tbl.h
index 493c5b6..a4658e5 100644
--- a/opcodes/arc-nps400-tbl.h
+++ b/opcodes/arc-nps400-tbl.h
@@ -11,3 +11,20 @@
 /* movb<.f><.cl> */
 { "movb", 0x48010000, 0xf80f8000, ARC_OPCODE_NPS400, BITOP, NONE, { NPS_R_DST_3B, NPS_R_SRC1_3B, NPS_R_SRC2_3B, NPS_BITOP_DST_POS, NPS_BITOP_SRC_POS, NPS_BITOP_SIZE }, { C_NPS_F }},
 { "movb", 0x48018000, 0xf80f8000, ARC_OPCODE_NPS400, BITOP, NONE, { NPS_R_DST_3B, NPS_R_SRC2_3B, NPS_BITOP_DST_POS, NPS_BITOP_SRC_POS, NPS_BITOP_SIZE }, { C_NPS_F, C_NPS_CL }},
+
+/****                  Pipeline Control Instructions                  ****/
+
+/* schd<.rw|.rd> */
+{ "schd", 0x3e6f7004, 0xffffff7f, ARC_OPCODE_NPS400, CONTROL, NONE, { 0 }, { C_NPS_SCHD_RW }},
+
+/* schd.wft.<.ie1|.ie2|.ie12> */
+{ "schd", 0x3e6f7044, 0xfffffcff, ARC_OPCODE_NPS400, CONTROL, NONE, { 0 }, { C_NPS_SCHD_TRIG, C_NPS_SCHD_IE }},
+
+/* sync<.rd|.wr> */
+{ "sync", 0x3e6f703f, 0xffffffbf, ARC_OPCODE_NPS400, CONTROL, NONE, { 0 }, { C_NPS_SYNC }},
+
+/* hwscd.off B */
+{ "hwschd", 0x386f00bf, 0xf8ff8fff, ARC_OPCODE_NPS400, CONTROL, NONE, { RB }, { C_NPS_HWS_OFF }},
+
+/* hwscd.restore 0,C */
+{ "hwschd", 0x3e6f7003, 0xfffff03f, ARC_OPCODE_NPS400, CONTROL, NONE, { ZA, RC }, { C_NPS_HWS_RESTORE }},
diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
index 231ef4b..da530fe 100644
--- a/opcodes/arc-opc.c
+++ b/opcodes/arc-opc.c
@@ -903,6 +903,37 @@ const struct arc_flag_operand arc_flag_operands[] =
 
 #define F_NPS_FLAG (F_NPS_CL + 1)
   { "f", 1, 1, 20, 1 },
+
+#define F_NPS_RW     (F_NPS_FLAG + 1)
+  { "rw", 0, 1, 7, 1 },
+
+#define F_NPS_RD     (F_NPS_RW + 1)
+  { "rd", 1, 1, 7, 1 },
+
+#define F_NPS_WFT     (F_NPS_RD + 1)
+  { "wft", 0, 0, 0, 1 },
+
+#define F_NPS_IE1     (F_NPS_WFT + 1)
+  { "ie1", 1, 2, 8, 1 },
+
+#define F_NPS_IE2     (F_NPS_IE1 + 1)
+  { "ie2", 2, 2, 8, 1 },
+
+#define F_NPS_IE12     (F_NPS_IE2 + 1)
+  { "ie12", 3, 2, 8, 1 },
+
+#define F_NPS_SYNC_RD     (F_NPS_IE12 + 1)
+  { "rd", 0, 1, 6, 1 },
+
+#define F_NPS_SYNC_WR     (F_NPS_SYNC_RD + 1)
+  { "wr", 1, 1, 6, 1 },
+
+#define F_NPS_HWS_OFF     (F_NPS_SYNC_WR + 1)
+  { "off", 0, 0, 0, 1 },
+
+#define F_NPS_HWS_RESTORE     (F_NPS_HWS_OFF + 1)
+  { "restore", 0, 0, 0, 1 },
+
 };
 
 const unsigned arc_num_flag_operands = ARRAY_SIZE (arc_flag_operands);
@@ -981,6 +1012,25 @@ const struct arc_flag_class arc_flag_classes[] =
 
 #define C_NPS_F     (C_NPS_CL + 1)
   { F_CLASS_OPTIONAL, { F_NPS_FLAG, F_NULL}},
+
+#define C_NPS_SCHD_RW     (C_NPS_F + 1)
+  { F_CLASS_REQUIRED, { F_NPS_RW, F_NPS_RD, F_NULL}},
+
+#define C_NPS_SCHD_TRIG     (C_NPS_SCHD_RW + 1)
+  { F_CLASS_REQUIRED, { F_NPS_WFT, F_NULL}},
+
+#define C_NPS_SCHD_IE     (C_NPS_SCHD_TRIG + 1)
+  { F_CLASS_OPTIONAL, { F_NPS_IE1, F_NPS_IE2, F_NPS_IE12, F_NULL}},
+
+#define C_NPS_SYNC     (C_NPS_SCHD_IE + 1)
+  { F_CLASS_REQUIRED, { F_NPS_SYNC_RD, F_NPS_SYNC_WR, F_NULL}},
+
+#define C_NPS_HWS_OFF     (C_NPS_SYNC + 1)
+  { F_CLASS_REQUIRED, { F_NPS_HWS_OFF, F_NULL}},
+
+#define C_NPS_HWS_RESTORE     (C_NPS_HWS_OFF + 1)
+  { F_CLASS_REQUIRED, { F_NPS_HWS_RESTORE, F_NULL}},
+
 };
 
 /* The operands table.
-- 
2.5.1

RE: [PATCH 6/6] arc/nps400: Add new control instructions

[Claudiu Zissulescu]

This is also ok.

Cheers,
Claudiu

> -----Original Message-----
> From: Andrew Burgess [mailto:andrew.burgess@embecosm.com]
> Sent: Sunday, April 03, 2016 12:51 AM
> To: binutils@sourceware.org
> Cc: Claudiu.Zissulescu@synopsys.com; noamca@mellanox.com; Andrew
> Burgess
> Subject: [PATCH 6/6] arc/nps400: Add new control instructions
> 
> Add some new control instructions to the opcodes library, and a new test
> for these new instructions to the assembler.  The new instructions use
> an instruction flag longer than any seen before (on arc), and so the max
> flag length is extended to accommodate this.
> 
> gas/ChangeLog:
> 
> 	* config/tc-arc.h (MAX_FLAG_NAME_LENGTH): Increase to 7.
> 	* testsuite/gas/arc/nps400-2.d: New file.
> 	* testsuite/gas/arc/nps400-2.s: New file.
> 
> opcodes/ChangeLog:
> 
> 	* arc-nps400-tbl.h: Add schd, sync, and hwschd instructions.
> 	* arc-opc.c (arc_flag_operands): Add new flags.
> 	(arc_flag_classes): Add new classes.
> ---
>  gas/ChangeLog                    |  6 +++++
>  gas/config/tc-arc.h              |  2 +-
>  gas/testsuite/gas/arc/nps400-2.d | 18 +++++++++++++++
>  gas/testsuite/gas/arc/nps400-2.s | 16 +++++++++++++
>  opcodes/ChangeLog                |  6 +++++
>  opcodes/arc-nps400-tbl.h         | 17 ++++++++++++++
>  opcodes/arc-opc.c                | 50
> ++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 114 insertions(+), 1 deletion(-)
>  create mode 100644 gas/testsuite/gas/arc/nps400-2.d
>  create mode 100644 gas/testsuite/gas/arc/nps400-2.s
> 
> diff --git a/gas/config/tc-arc.h b/gas/config/tc-arc.h
> index 93a7d77..7bd281c 100644
> --- a/gas/config/tc-arc.h
> +++ b/gas/config/tc-arc.h
> @@ -210,7 +210,7 @@ extern int tc_arc_regname_to_dw2regnum (char
> *regname);
>  #define NOP_OPCODE_S   0x000078E0
>  #define NOP_OPCODE_L   0x264A7000 /* mov 0,0.  */
> 
> -#define MAX_FLAG_NAME_LENGTH 3
> +#define MAX_FLAG_NAME_LENGTH 7
> 
>  struct arc_flags
>  {
> diff --git a/gas/testsuite/gas/arc/nps400-2.d b/gas/testsuite/gas/arc/nps400-
> 2.d
> new file mode 100644
> index 0000000..966907e
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/nps400-2.d
> @@ -0,0 +1,18 @@
> +#as: -mcpu=nps400
> +#objdump: -dr
> +
> +.*: +file format .*arc.*
> +
> +Disassembly of section .text:
> +
> +[0-9a-f]+ <.*>:
> +   0:	3e6f 7004           	schd\.rw
> +   4:	3e6f 7084           	schd\.rd
> +   8:	3e6f 7044           	schd\.wft
> +   c:	3e6f 7144           	schd\.wft\.ie1
> +  10:	3e6f 7244           	schd\.wft\.ie2
> +  14:	3e6f 7344           	schd\.wft\.ie12
> +  18:	3e6f 703f           	sync\.rd
> +  1c:	3e6f 707f           	sync\.wr
> +  20:	3a6f 10bf           	hwschd\.off	r10
> +  24:	3e6f 7503           	hwschd\.restore	0,r20
> diff --git a/gas/testsuite/gas/arc/nps400-2.s b/gas/testsuite/gas/arc/nps400-
> 2.s
> new file mode 100644
> index 0000000..de4c839
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/nps400-2.s
> @@ -0,0 +1,16 @@
> +        .text
> +        ;; schd
> +        schd.rw
> +        schd.rd
> +        schd.wft
> +        schd.wft.ie1
> +        schd.wft.ie2
> +        schd.wft.ie12
> +
> +        ;; sync
> +        sync.rd
> +        sync.wr
> +
> +        ;; hwschd
> +        hwschd.off      r10
> +        hwschd.restore  0, r20
> diff --git a/opcodes/arc-nps400-tbl.h b/opcodes/arc-nps400-tbl.h
> index 493c5b6..a4658e5 100644
> --- a/opcodes/arc-nps400-tbl.h
> +++ b/opcodes/arc-nps400-tbl.h
> @@ -11,3 +11,20 @@
>  /* movb<.f><.cl> */
>  { "movb", 0x48010000, 0xf80f8000, ARC_OPCODE_NPS400, BITOP, NONE, {
> NPS_R_DST_3B, NPS_R_SRC1_3B, NPS_R_SRC2_3B, NPS_BITOP_DST_POS,
> NPS_BITOP_SRC_POS, NPS_BITOP_SIZE }, { C_NPS_F }},
>  { "movb", 0x48018000, 0xf80f8000, ARC_OPCODE_NPS400, BITOP, NONE, {
> NPS_R_DST_3B, NPS_R_SRC2_3B, NPS_BITOP_DST_POS,
> NPS_BITOP_SRC_POS, NPS_BITOP_SIZE }, { C_NPS_F, C_NPS_CL }},
> +
> +/****                  Pipeline Control Instructions                  ****/
> +
> +/* schd<.rw|.rd> */
> +{ "schd", 0x3e6f7004, 0xffffff7f, ARC_OPCODE_NPS400, CONTROL, NONE, {
> 0 }, { C_NPS_SCHD_RW }},
> +
> +/* schd.wft.<.ie1|.ie2|.ie12> */
> +{ "schd", 0x3e6f7044, 0xfffffcff, ARC_OPCODE_NPS400, CONTROL, NONE, {
> 0 }, { C_NPS_SCHD_TRIG, C_NPS_SCHD_IE }},
> +
> +/* sync<.rd|.wr> */
> +{ "sync", 0x3e6f703f, 0xffffffbf, ARC_OPCODE_NPS400, CONTROL, NONE, {
> 0 }, { C_NPS_SYNC }},
> +
> +/* hwscd.off B */
> +{ "hwschd", 0x386f00bf, 0xf8ff8fff, ARC_OPCODE_NPS400, CONTROL,
> NONE, { RB }, { C_NPS_HWS_OFF }},
> +
> +/* hwscd.restore 0,C */
> +{ "hwschd", 0x3e6f7003, 0xfffff03f, ARC_OPCODE_NPS400, CONTROL,
> NONE, { ZA, RC }, { C_NPS_HWS_RESTORE }},
> diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
> index 231ef4b..da530fe 100644
> --- a/opcodes/arc-opc.c
> +++ b/opcodes/arc-opc.c
> @@ -903,6 +903,37 @@ const struct arc_flag_operand arc_flag_operands[] =
> 
>  #define F_NPS_FLAG (F_NPS_CL + 1)
>    { "f", 1, 1, 20, 1 },
> +
> +#define F_NPS_RW     (F_NPS_FLAG + 1)
> +  { "rw", 0, 1, 7, 1 },
> +
> +#define F_NPS_RD     (F_NPS_RW + 1)
> +  { "rd", 1, 1, 7, 1 },
> +
> +#define F_NPS_WFT     (F_NPS_RD + 1)
> +  { "wft", 0, 0, 0, 1 },
> +
> +#define F_NPS_IE1     (F_NPS_WFT + 1)
> +  { "ie1", 1, 2, 8, 1 },
> +
> +#define F_NPS_IE2     (F_NPS_IE1 + 1)
> +  { "ie2", 2, 2, 8, 1 },
> +
> +#define F_NPS_IE12     (F_NPS_IE2 + 1)
> +  { "ie12", 3, 2, 8, 1 },
> +
> +#define F_NPS_SYNC_RD     (F_NPS_IE12 + 1)
> +  { "rd", 0, 1, 6, 1 },
> +
> +#define F_NPS_SYNC_WR     (F_NPS_SYNC_RD + 1)
> +  { "wr", 1, 1, 6, 1 },
> +
> +#define F_NPS_HWS_OFF     (F_NPS_SYNC_WR + 1)
> +  { "off", 0, 0, 0, 1 },
> +
> +#define F_NPS_HWS_RESTORE     (F_NPS_HWS_OFF + 1)
> +  { "restore", 0, 0, 0, 1 },
> +
>  };
> 
>  const unsigned arc_num_flag_operands = ARRAY_SIZE (arc_flag_operands);
> @@ -981,6 +1012,25 @@ const struct arc_flag_class arc_flag_classes[] =
> 
>  #define C_NPS_F     (C_NPS_CL + 1)
>    { F_CLASS_OPTIONAL, { F_NPS_FLAG, F_NULL}},
> +
> +#define C_NPS_SCHD_RW     (C_NPS_F + 1)
> +  { F_CLASS_REQUIRED, { F_NPS_RW, F_NPS_RD, F_NULL}},
> +
> +#define C_NPS_SCHD_TRIG     (C_NPS_SCHD_RW + 1)
> +  { F_CLASS_REQUIRED, { F_NPS_WFT, F_NULL}},
> +
> +#define C_NPS_SCHD_IE     (C_NPS_SCHD_TRIG + 1)
> +  { F_CLASS_OPTIONAL, { F_NPS_IE1, F_NPS_IE2, F_NPS_IE12, F_NULL}},
> +
> +#define C_NPS_SYNC     (C_NPS_SCHD_IE + 1)
> +  { F_CLASS_REQUIRED, { F_NPS_SYNC_RD, F_NPS_SYNC_WR, F_NULL}},
> +
> +#define C_NPS_HWS_OFF     (C_NPS_SYNC + 1)
> +  { F_CLASS_REQUIRED, { F_NPS_HWS_OFF, F_NULL}},
> +
> +#define C_NPS_HWS_RESTORE     (C_NPS_HWS_OFF + 1)
> +  { F_CLASS_REQUIRED, { F_NPS_HWS_RESTORE, F_NULL}},
> +
>  };
> 
>  /* The operands table.
> --
> 2.5.1

Re: [PATCH 6/6] arc/nps400: Add new control instructions

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.h (MAX_FLAG_NAME_LENGTH): Increase to 7.
> 	* testsuite/gas/arc/nps400-2.d: New file.
> 	* testsuite/gas/arc/nps400-2.s: New file.
> 
> opcodes/ChangeLog:
> 
> 	* arc-nps400-tbl.h: Add schd, sync, and hwschd instructions.
> 	* arc-opc.c (arc_flag_operands): Add new flags.
> 	(arc_flag_classes): Add new classes.
 
Approved - please apply.

Cheers
  Nick

RE: [PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[Claudiu Zissulescu]

Hi Andrew,

> The assembler builds a hash to hold references to arc_opcode entries in
> the arc_opcodes table.  This hash assumes that each mnemonic will always
> appear in contiguous blocks within the arc_opcodes table, so all ADD
> instruction will be together, all AND instructions will likewise be
> together and so on.

Yes this is on purpose. Having the group, we ensure we choose the correct encoding. As you can see the grouping of a mnemonic is done accordingly to an order, such that the small immediate arguments are preferred before the long ones. Once, breaking that you need to do the sorting at runtime, which will increase the time a file is assembled. 

> 
> The problem with this is that as different variations of arc are added,
> then it is often more convenient to split instructions apart, so all the
> base ADD instructions are together, but, variants of ADD specific to one
> variation of arc are grouped with other instructions specific to that
> arc variant.  The current data structures don't support this
> configuration of instructions.

It may not be a good idea to overwrite the basic ARC ISA. Any ARC customer can choose to have a different name than the standard ISA names. Please can u reconsider this. I do not see any reason why a customer to use ADD mnemonic and not MYADD or something of a sort. Allowing the overwriting/addnotation of the standard ISA mnemonics is opening a domain of pain. On top of everything, I miss tests for your proposed patch (1 to 4).

Best,
Claudiu

Re: [PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[Andrew Burgess]

* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-04-04 08:21:11 +0000]:

> Hi Andrew,
> 
> > The assembler builds a hash to hold references to arc_opcode entries in
> > the arc_opcodes table.  This hash assumes that each mnemonic will always
> > appear in contiguous blocks within the arc_opcodes table, so all ADD
> > instruction will be together, all AND instructions will likewise be
> > together and so on.
> 
> Yes this is on purpose. Having the group, we ensure we choose the
> correct encoding. As you can see the grouping of a mnemonic is done
> accordingly to an order, such that the small immediate arguments are
> preferred before the long ones. Once, breaking that you need to do
> the sorting at runtime, which will increase the time a file is
> assembled.

I see what you mean, though I slightly disagree, it's the ordering,
not the grouping that is relied upon.  So long as any second group
only supports instructions that are not covered by an earlier group
then all will be fine.

Given that the arc assembler as currently written relies on the
programmer to get the ordering "just right" I don't (personally) see
restricting to one group as being that much of an advantage in getting
the ordering correct.  I guess you disagree, but

> 
> > 
> > The problem with this is that as different variations of arc are added,
> > then it is often more convenient to split instructions apart, so all the
> > base ADD instructions are together, but, variants of ADD specific to one
> > variation of arc are grouped with other instructions specific to that
> > arc variant.  The current data structures don't support this
> > configuration of instructions.
> 
> It may not be a good idea to overwrite the basic ARC ISA. Any ARC
> customer can choose to have a different name than the standard ISA
> names. Please can u reconsider this.

I don't really understand what you are suggesting here, but ...

>                                        I do not see any reason why a
> customer to use ADD mnemonic and not MYADD or something of a
> sort.

... I think you're saying, change the ISA to work around limitation in
the assembler :-/ That doesn't feel like a good answer, surely we
should just adapt the assembler until it is flexible enough to handle
whatever ISAs we need.

>       Allowing the overwriting/addnotation of the standard ISA
> mnemonics is opening a domain of pain.

On inspection you'll notice that the new instruction actually uses a
unique flag operand and so there will never be a chance of confusion
in this case.

Could you clarify who you think will suffer this "domain of pain"?
The tool-chain maintainer?  The customer of Synopsys who is adding the
new instructions?  The user of the final tools?  Or someone else?

In the example you gave above, if a customer can think of a new ADD
instruction that fills a gap not covered by the core ADD instruction
then I think they absolutely _should_ call it ADD, not MYADD.

Consider the end user of the tools, if this new ADD is just like
any other ADD, but happens to take different parameter types, then
having this one variant being called MYADD would be highly annoying,
and would feel like the instruction was glued onto the side of the
architecture, rather than being seamlessly integrated into it.  I
think that we can, and should, do better.

>                                         On top of everything, I miss
> tests for your proposed patch (1 to 4).

Sorry I was not explicit in the commit message, but there should be no
functional change from patches 1 through 4.  Patch 5 should have no
functional change given the current state of the assembler.  These
tests are therefore that everything that currently works should
continue to do so.

It is only in patch 6 where I add a new instruction (that relies on
patches 1 to 5) that I add a new test.

I think I see 3 possible ways forward:

  1. Keep the current patch series, understanding that the ordering
  restriction still applies even across disjoint groups of
  instructions, and accepting that as a know danger area in the
  assembler.  This would be my preferred solution.

  2. Move this one nps instruction (sync.{rd,wr}) into the main arc
  opcode table, placing it next the core 'sync' instruction.  I'd
  probably leave a comment in the nps decode table explaining this
  strange placement.

  3. Write a small program that reads in the arc decode tables and
  places the entries into a single unified order, which can then be
  read by the assembler and disassembler.

Interested to hear your thoughts,

thanks,
Andrew

RE: [PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[Claudiu Zissulescu]

> > > The problem with this is that as different variations of arc are added,
> > > then it is often more convenient to split instructions apart, so all the
> > > base ADD instructions are together, but, variants of ADD specific to one
> > > variation of arc are grouped with other instructions specific to that
> > > arc variant.  The current data structures don't support this
> > > configuration of instructions.
> >
> > It may not be a good idea to overwrite the basic ARC ISA. Any ARC
> > customer can choose to have a different name than the standard ISA
> > names. Please can u reconsider this.
> 
> I don't really understand what you are suggesting here, but ...
> 
> >                                        I do not see any reason why a
> > customer to use ADD mnemonic and not MYADD or something of a
> > sort.
> 
> ... I think you're saying, change the ISA to work around limitation in
> the assembler :-/ That doesn't feel like a good answer, surely we
> should just adapt the assembler until it is flexible enough to handle
> whatever ISAs we need.

What I am trying to say is that the ADD instruction should be as described in ARC's Programmer Reference Manual, and we shouldn't invent new mnemonics formats. Maybe, you pick wrongly the ADD example, and you don't intend to overwrite the standard ADD mnemonics format, which may break other tools as well (e.g. gcc).
As far as I understand, your patch is about the possibility of describing a 3rd party instruction in separate groups, which is fine by me.

> 
> >       Allowing the overwriting/addnotation of the standard ISA
> > mnemonics is opening a domain of pain.
> 
> Could you clarify who you think will suffer this "domain of pain"?

Mainly, the tool chain maintainer, which needs to keep an eye over the compatibility between the GNU toolchain and the in-house ARC toolchain (we are not alone ;)). 
 
>   1. Keep the current patch series, understanding that the ordering
>   restriction still applies even across disjoint groups of
>   instructions, and accepting that as a know danger area in the
>   assembler.  This would be my preferred solution.

From my side, the idea of the patch is ok. You need to add the explanation about keeping the ordering even if you use multiple groups somewhere in your patch (probably in tc-arc.c).

Cheers,
Claudiu

Re: [PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[Andrew Burgess]

* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-04-04 14:43:15 +0000]:

> > > > The problem with this is that as different variations of arc are added,
> > > > then it is often more convenient to split instructions apart, so all the
> > > > base ADD instructions are together, but, variants of ADD specific to one
> > > > variation of arc are grouped with other instructions specific to that
> > > > arc variant.  The current data structures don't support this
> > > > configuration of instructions.
> > >
> > > It may not be a good idea to overwrite the basic ARC ISA. Any ARC
> > > customer can choose to have a different name than the standard ISA
> > > names. Please can u reconsider this.
> > 
> > I don't really understand what you are suggesting here, but ...
> > 
> > >                                        I do not see any reason why a
> > > customer to use ADD mnemonic and not MYADD or something of a
> > > sort.
> > 
> > ... I think you're saying, change the ISA to work around limitation in
> > the assembler :-/ That doesn't feel like a good answer, surely we
> > should just adapt the assembler until it is flexible enough to handle
> > whatever ISAs we need.
> 
> What I am trying to say is that the ADD instruction should be as
> described in ARC's Programmer Reference Manual, and we shouldn't
> invent new mnemonics formats. Maybe, you pick wrongly the ADD
> example, and you don't intend to overwrite the standard ADD
> mnemonics format, which may break other tools as well (e.g. gcc).
> As far as I understand, your patch is about the possibility of
> describing a 3rd party instruction in separate groups, which is fine
> by me.

I think you're covering a lot of ground here, some of which I agree
(strongly) with you on, but I don't think are what I'm proposing.

Let's consider ARC700 for now, it has a specific set of ADD
instructions defined.

Now I'm _adding_ extension instructions.  If one of my extensions
instructions happened to be an addition instruction that took 4
registers, such that DST = SRC1 + SRC2 + SRC3, then this is clearly
_not_ part of the core ARC700 architecture, but could live quite
happily as an extension to ARC700.

If I wanted to make an extension to ARC700 that actually rendered some
of the core ARC700 instruction formats invalid, or gave them a
different encoding then (a) I agree that's probably a bad idea, and
(b) I would argue that stops being an extension to ARC700, and becomes
a new architecture variant that is similar too, but is _not_ an
extension of ARC700.

What I want to do here is make an extension.  Unless I teach GCC about
the new instructions then GCC will never use them, but neither will
anything produced by GCC fail to assemble, all the core instructions
are still valid.

> 
> > 
> > >       Allowing the overwriting/addnotation of the standard ISA
> > > mnemonics is opening a domain of pain.
> > 
> > Could you clarify who you think will suffer this "domain of pain"?
> 
> Mainly, the tool chain maintainer, which needs to keep an eye over
> the compatibility between the GNU toolchain and the in-house ARC
> toolchain (we are not alone ;)).

Sure, but I suspect you guys will not be rushing to add nps support to
your in house tools.  If I (or anyone else) puts forward a patch that
changes core arc700 functionality then that patch is broken.  If the
patch makes it into master then it needs fixing / reverting and more
tests adding.

As all these new instructions are extensions there should be no risk
of compatibility problems.  Of course we need to be careful in review,
but that's always true :)

>  
> >   1. Keep the current patch series, understanding that the ordering
> >   restriction still applies even across disjoint groups of
> >   instructions, and accepting that as a know danger area in the
> >   assembler.  This would be my preferred solution.
> 
> From my side, the idea of the patch is ok. You need to add the
> explanation about keeping the ordering even if you use multiple
> groups somewhere in your patch (probably in tc-arc.c).

OK, that sounds like you're basically happy with the patch as is
then.  I'll update the patch series and post a new version here real
soon.

In the end I placed the comment into opcodes/arc-opc.c, at the head of
the arc_opcodes table.  I figured that anyone interested in adding new
instructions would go there first, and any comment there was most
likely to be found / read.

I look forward to any further feedback you have.

Thanks,
Andrew

RE: [PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[Claudiu Zissulescu]

> Now I'm _adding_ extension instructions.  If one of my extensions
> instructions happened to be an addition instruction that took 4
> registers, such that DST = SRC1 + SRC2 + SRC3, then this is clearly
> _not_ part of the core ARC700 architecture, but could live quite
> happily as an extension to ARC700.

My apologizes, I thought you would like to overwrite the current mnemonic syntax. I agree we can have extra syntax for cases which are like the one you pointed out.

> In the end I placed the comment into opcodes/arc-opc.c, at the head of
> the arc_opcodes table.  I figured that anyone interested in adding new
> instructions would go there first, and any comment there was most
> likely to be found / read.

Nice.

Please go ahead. I am waiting for your patches to be upstreamed, as I have three patches which I would like to upstream after your commit (support for .extInstruction pseudo-ops).

Thanks,
Claudiu

Re: [PATCH 1/6] gas/arc: Modify structure used to hold opcodes

[Nick Clifton]

Hi Andrew,

Just to second Claudiu's approval - please go ahead and apply this patch.

Cheers
  Nick

[PATCHv2 4/4] arc/nps400: Add new instructions

[Andrew Burgess]

Add some new control instructions to the opcodes library, and a new test
for these new instructions to the assembler.  The new instructions use
an instruction flag longer than any seen before (on arc), and so the max
flag length is extended to accommodate this.

gas/ChangeLog:

	* config/tc-arc.h (MAX_FLAG_NAME_LENGTH): Increase to 7.
	* testsuite/gas/arc/nps400-2.d: New file.
	* testsuite/gas/arc/nps400-2.s: New file.

opcodes/ChangeLog:

	* arc-nps400-tbl.h: Add schd, sync, and hwschd instructions.
	* arc-opc.c (arc_flag_operands): Add new flags.
	(arc_flag_classes): Add new classes.
---
 gas/ChangeLog                    |  6 +++++
 gas/config/tc-arc.h              |  2 +-
 gas/testsuite/gas/arc/nps400-2.d | 18 +++++++++++++++
 gas/testsuite/gas/arc/nps400-2.s | 16 +++++++++++++
 opcodes/ChangeLog                |  6 +++++
 opcodes/arc-nps400-tbl.h         | 17 ++++++++++++++
 opcodes/arc-opc.c                | 50 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 114 insertions(+), 1 deletion(-)
 create mode 100644 gas/testsuite/gas/arc/nps400-2.d
 create mode 100644 gas/testsuite/gas/arc/nps400-2.s

diff --git a/gas/config/tc-arc.h b/gas/config/tc-arc.h
index 7c193fe..076afea 100644
--- a/gas/config/tc-arc.h
+++ b/gas/config/tc-arc.h
@@ -211,7 +211,7 @@ extern int tc_arc_regname_to_dw2regnum (char *regname);
 #define NOP_OPCODE_S   0x000078E0
 #define NOP_OPCODE_L   0x264A7000 /* mov 0,0.  */
 
-#define MAX_FLAG_NAME_LENGTH 3
+#define MAX_FLAG_NAME_LENGTH 7
 
 struct arc_flags
 {
diff --git a/gas/testsuite/gas/arc/nps400-2.d b/gas/testsuite/gas/arc/nps400-2.d
new file mode 100644
index 0000000..966907e
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-2.d
@@ -0,0 +1,18 @@
+#as: -mcpu=nps400
+#objdump: -dr
+
+.*: +file format .*arc.*
+
+Disassembly of section .text:
+
+[0-9a-f]+ <.*>:
+   0:	3e6f 7004           	schd\.rw
+   4:	3e6f 7084           	schd\.rd
+   8:	3e6f 7044           	schd\.wft
+   c:	3e6f 7144           	schd\.wft\.ie1
+  10:	3e6f 7244           	schd\.wft\.ie2
+  14:	3e6f 7344           	schd\.wft\.ie12
+  18:	3e6f 703f           	sync\.rd
+  1c:	3e6f 707f           	sync\.wr
+  20:	3a6f 10bf           	hwschd\.off	r10
+  24:	3e6f 7503           	hwschd\.restore	0,r20
diff --git a/gas/testsuite/gas/arc/nps400-2.s b/gas/testsuite/gas/arc/nps400-2.s
new file mode 100644
index 0000000..de4c839
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-2.s
@@ -0,0 +1,16 @@
+        .text
+        ;; schd
+        schd.rw
+        schd.rd
+        schd.wft
+        schd.wft.ie1
+        schd.wft.ie2
+        schd.wft.ie12
+
+        ;; sync
+        sync.rd
+        sync.wr
+
+        ;; hwschd
+        hwschd.off      r10
+        hwschd.restore  0, r20
diff --git a/opcodes/arc-nps400-tbl.h b/opcodes/arc-nps400-tbl.h
index dc7b066..58d479f 100644
--- a/opcodes/arc-nps400-tbl.h
+++ b/opcodes/arc-nps400-tbl.h
@@ -123,3 +123,20 @@
 
 /* crc32<.r> 0,limm,u6		00111 110 01 110100 R 111 uuuuuu 111110 */
 { "crc32", 0x3e74703e, 0xffff703f, ARC_OPCODE_NPS400, BITOP, NONE, { ZA, LIMM, UIMM6_20 }, { C_NPS_R }},
+
+/****                  Pipeline Control Instructions                  ****/
+
+/* schd<.rw|.rd> */
+{ "schd", 0x3e6f7004, 0xffffff7f, ARC_OPCODE_NPS400, CONTROL, NONE, { 0 }, { C_NPS_SCHD_RW }},
+
+/* schd.wft.<.ie1|.ie2|.ie12> */
+{ "schd", 0x3e6f7044, 0xfffffcff, ARC_OPCODE_NPS400, CONTROL, NONE, { 0 }, { C_NPS_SCHD_TRIG, C_NPS_SCHD_IE }},
+
+/* sync<.rd|.wr> */
+{ "sync", 0x3e6f703f, 0xffffffbf, ARC_OPCODE_NPS400, CONTROL, NONE, { 0 }, { C_NPS_SYNC }},
+
+/* hwscd.off B */
+{ "hwschd", 0x386f00bf, 0xf8ff8fff, ARC_OPCODE_NPS400, CONTROL, NONE, { RB }, { C_NPS_HWS_OFF }},
+
+/* hwscd.restore 0,C */
+{ "hwschd", 0x3e6f7003, 0xfffff03f, ARC_OPCODE_NPS400, CONTROL, NONE, { ZA, RC }, { C_NPS_HWS_RESTORE }},
diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
index 69c65fc..d667a78 100644
--- a/opcodes/arc-opc.c
+++ b/opcodes/arc-opc.c
@@ -1003,6 +1003,37 @@ const struct arc_flag_operand arc_flag_operands[] =
 
 #define F_NPS_R     (F_NPS_FLAG + 1)
   { "r",  1, 1, 15, 1 },
+
+#define F_NPS_RW     (F_NPS_R + 1)
+  { "rw", 0, 1, 7, 1 },
+
+#define F_NPS_RD     (F_NPS_RW + 1)
+  { "rd", 1, 1, 7, 1 },
+
+#define F_NPS_WFT     (F_NPS_RD + 1)
+  { "wft", 0, 0, 0, 1 },
+
+#define F_NPS_IE1     (F_NPS_WFT + 1)
+  { "ie1", 1, 2, 8, 1 },
+
+#define F_NPS_IE2     (F_NPS_IE1 + 1)
+  { "ie2", 2, 2, 8, 1 },
+
+#define F_NPS_IE12     (F_NPS_IE2 + 1)
+  { "ie12", 3, 2, 8, 1 },
+
+#define F_NPS_SYNC_RD     (F_NPS_IE12 + 1)
+  { "rd", 0, 1, 6, 1 },
+
+#define F_NPS_SYNC_WR     (F_NPS_SYNC_RD + 1)
+  { "wr", 1, 1, 6, 1 },
+
+#define F_NPS_HWS_OFF     (F_NPS_SYNC_WR + 1)
+  { "off", 0, 0, 0, 1 },
+
+#define F_NPS_HWS_RESTORE     (F_NPS_HWS_OFF + 1)
+  { "restore", 0, 0, 0, 1 },
+
 };
 
 const unsigned arc_num_flag_operands = ARRAY_SIZE (arc_flag_operands);
@@ -1084,6 +1115,25 @@ const struct arc_flag_class arc_flag_classes[] =
 
 #define C_NPS_R     (C_NPS_F + 1)
   { F_CLASS_OPTIONAL, { F_NPS_R, F_NULL}},
+
+#define C_NPS_SCHD_RW     (C_NPS_R + 1)
+  { F_CLASS_REQUIRED, { F_NPS_RW, F_NPS_RD, F_NULL}},
+
+#define C_NPS_SCHD_TRIG     (C_NPS_SCHD_RW + 1)
+  { F_CLASS_REQUIRED, { F_NPS_WFT, F_NULL}},
+
+#define C_NPS_SCHD_IE     (C_NPS_SCHD_TRIG + 1)
+  { F_CLASS_OPTIONAL, { F_NPS_IE1, F_NPS_IE2, F_NPS_IE12, F_NULL}},
+
+#define C_NPS_SYNC     (C_NPS_SCHD_IE + 1)
+  { F_CLASS_REQUIRED, { F_NPS_SYNC_RD, F_NPS_SYNC_WR, F_NULL}},
+
+#define C_NPS_HWS_OFF     (C_NPS_SYNC + 1)
+  { F_CLASS_REQUIRED, { F_NPS_HWS_OFF, F_NULL}},
+
+#define C_NPS_HWS_RESTORE     (C_NPS_HWS_OFF + 1)
+  { F_CLASS_REQUIRED, { F_NPS_HWS_RESTORE, F_NULL}},
+
 };
 
 /* The operands table.
-- 
2.5.1

RE: [PATCHv2 4/4] arc/nps400: Add new instructions

[Claudiu Zissulescu]

> 	* config/tc-arc.h (MAX_FLAG_NAME_LENGTH): Increase to 7.

This is a good question: what should be the maximum size of a flag name, considering we can add new ones via .extCondCode. Probably 7 is alright. 

Anyhow, the patch looks alright to me,
Claudiu

Re: [PATCHv2 4/4] arc/nps400: Add new instructions

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.h (MAX_FLAG_NAME_LENGTH): Increase to 7.
> 	* testsuite/gas/arc/nps400-2.d: New file.
> 	* testsuite/gas/arc/nps400-2.s: New file.
> 
> opcodes/ChangeLog:
> 
> 	* arc-nps400-tbl.h: Add schd, sync, and hwschd instructions.
> 	* arc-opc.c (arc_flag_operands): Add new flags.
> 	(arc_flag_classes): Add new classes.
 
Approved - please apply.

Cheers
  Nick

[PATCHv2 1/4] gas/arc: Modify structure used to hold opcodes

[Andrew Burgess]

The arc assembler builds a hash table to hold references to arc_opcode
entries in the arc_opcodes table.  This hash assumes that each mnemonic
will always appear in a contiguous blocks within the arc_opcodes table;
all ADD instruction will be together, all AND instructions will likewise
be together and so on.

The problem with this is that as different variations of arc are added,
then it is often more convenient to split instructions apart, so all the
base ADD instructions are together, but, variants of ADD specific to one
variation of arc are grouped with other instructions specific to that
arc variant.  The current data structures don't support splitting the
instructions in this way.

This commit is a first step towards addressing this limitation.  In this
commit the hash table that currently holds arc_opcode pointers directly,
instead holds a pointer to a new, intermediate, data structure.  This
new data structure holds the pointer to the arc_opcode.  In this way, we
can, in the future support having the intermediate structure hold
multiple pointers to different arc_opcode groups.

There should be no visible functional change after this commit.

gas/ChangeLog:

	* config/tc-arc.c (struct arc_opcode_hash_entry): New structure.
	(arc_find_opcode): New function.
	(find_special_case_pseudo): Use arc_find_opcode.
	(find_special_case_flag): Likewise.
	(assemble_tokens): Likewise.
	(md_begin): Build hash using struct arc_opcode_hash_entry.
---
 gas/ChangeLog       |  9 ++++++++
 gas/config/tc-arc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 154e838..1dcf125 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -308,6 +308,16 @@ static struct arc_last_insn
   bfd_boolean has_delay_slot;
 } arc_last_insns[2];
 
+/* Structure to hold an entry in ARC_OPCODE_HASH.  */
+struct arc_opcode_hash_entry
+{
+  /* The number of pointers in the OPCODE list.  */
+  size_t count;
+
+  /* Points to a list of opcode pointers.  */
+  const struct arc_opcode **opcode;
+};
+
 /* Forward declaration.  */
 static void assemble_insn
   (const struct arc_opcode *, const expressionS *, int,
@@ -554,6 +564,29 @@ static bfd_boolean assembling_insn = FALSE;
 
 /* Functions implementation.  */
 
+/* Return a pointer to the first entry in ARC_OPCODE_HASH that matches
+   NAME, or NULL if there are no matching entries.  */
+
+static const struct arc_opcode *
+arc_find_opcode (const char *name)
+{
+  const struct arc_opcode_hash_entry *entry;
+  const struct arc_opcode *opcode;
+
+  entry = hash_find (arc_opcode_hash, name);
+  if (entry != NULL)
+    {
+      if (entry->count > 1)
+        as_fatal (_("unable to lookup `%s', too many opcode chains"),
+                  name);
+      opcode = *entry->opcode;
+    }
+  else
+    opcode = NULL;
+
+  return opcode;
+}
+
 /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
    is encoded as 'middle-endian' for a little-endian target.  FIXME!
    this function is used for regular 4 byte instructions as well.  */
@@ -1932,8 +1965,7 @@ find_special_case_pseudo (const char *opname,
       break;
     }
 
-  return (const struct arc_opcode *)
-    hash_find (arc_opcode_hash,	pseudo_insn->mnemonic_r);
+  return arc_find_opcode (pseudo_insn->mnemonic_r);
 }
 
 static const struct arc_opcode *
@@ -1969,9 +2001,7 @@ find_special_case_flag (const char *opname,
 	  flaglen = strlen (flagnm);
 	  if (strcmp (opname + oplen, flagnm) == 0)
 	    {
-	      opcode = (const struct arc_opcode *)
-		hash_find (arc_opcode_hash,
-			   arc_flag_special_opcode->name);
+              opcode = arc_find_opcode (arc_flag_special_opcode->name);
 
 	      if (*nflgs + 1 > MAX_INSN_FLGS)
 		break;
@@ -2019,7 +2049,7 @@ assemble_tokens (const char *opname,
   int cpumatch = 1;
 
   /* Search opcodes.  */
-  opcode = (const struct arc_opcode *) hash_find (arc_opcode_hash, opname);
+  opcode = arc_find_opcode (opname);
 
   /* Couldn't find opcode conventional way, try special cases.  */
   if (!opcode)
@@ -2164,12 +2194,28 @@ md_begin (void)
   for (i = 0; i < arc_num_opcodes;)
     {
       const char *name, *retval;
+      struct arc_opcode_hash_entry *entry;
 
       name = arc_opcodes[i].name;
-      retval = hash_insert (arc_opcode_hash, name, (void *) &arc_opcodes[i]);
-      if (retval)
-	as_fatal (_("internal error: can't hash opcode '%s': %s"),
-		  name, retval);
+
+      entry = hash_find (arc_opcode_hash, name);
+      if (entry == NULL)
+        {
+          entry = xmalloc (sizeof (*entry));
+          entry->count = 0;
+          entry->opcode = NULL;
+
+          retval = hash_insert (arc_opcode_hash, name, (void *) entry);
+          if (retval)
+            as_fatal (_("internal error: can't hash opcode '%s': %s"),
+                      name, retval);
+        }
+
+      entry->opcode = xrealloc (entry->opcode,
+                                sizeof (const struct arc_opcode *)
+                                * entry->count + 1);
+      entry->opcode [entry->count] = &arc_opcodes[i];
+      entry->count++;
 
       while (++i < arc_num_opcodes
 	     && (arc_opcodes[i].name == name
-- 
2.5.1

RE: [PATCHv2 1/4] gas/arc: Modify structure used to hold opcodes

[Claudiu Zissulescu]

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (struct arc_opcode_hash_entry): New structure.
> 	(arc_find_opcode): New function.
> 	(find_special_case_pseudo): Use arc_find_opcode.
> 	(find_special_case_flag): Likewise.
> 	(assemble_tokens): Likewise.
> 	(md_begin): Build hash using struct arc_opcode_hash_entry.
> -

Ok
Cheers,
Claudiu

Re: [PATCHv2 1/4] gas/arc: Modify structure used to hold opcodes

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (struct arc_opcode_hash_entry): New structure.
> 	(arc_find_opcode): New function.
> 	(find_special_case_pseudo): Use arc_find_opcode.
> 	(find_special_case_flag): Likewise.
> 	(assemble_tokens): Likewise.
> 	(md_begin): Build hash using struct arc_opcode_hash_entry.
 
Approved - please apply.

Cheers
  Nick

[PATCHv2 3/4] gas/arc: Handle multiple arc_opcode chains for same mnemonic

[Andrew Burgess]

This commit completes support for having multiple instructions with the
same mnemonic in non-contiguous blocks within the arc_opcodes table.

The commit adds an iterator mechanism for the arc_opcode_hash_entry
structure, which is then used in find_opcode_match to consider all
arc_opcode entries with the same mnemonic, even when these instructions
are stored in non-contiguous blocks.

I extend the comment on the arc_opcodes table to discuss how entries
within the table are organised, and to mention how instructions can be
split into multiple groups if needed, but that the table is still
searched in table order.

There should be no user visible changes after this commit.

gas/ChangeLog:

	* config/tc-arc.c (struct arc_opcode_hash_entry_iterator): New
	structure.
	(arc_opcode_hash_entry_iterator_init): New function.
	(arc_opcode_hash_entry_iterator_next): New function.
	(find_opcode_match): Iterate over all arc_opcode entries
	referenced by the arc_opcode_hash_entry passed in as a parameter.

opcodes/ChangeLog:

	* arc-opc.c (arc_opcodes): Extend comment to discus table layout.
---
 gas/ChangeLog       |  9 +++++++
 gas/config/tc-arc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++---------
 opcodes/ChangeLog   |  4 +++
 opcodes/arc-opc.c   | 32 +++++++++++++++++++++++-
 4 files changed, 104 insertions(+), 13 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 5b76a07..7efc630 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -318,6 +318,17 @@ struct arc_opcode_hash_entry
   const struct arc_opcode **opcode;
 };
 
+/* Structure used for iterating through an arc_opcode_hash_entry.  */
+struct arc_opcode_hash_entry_iterator
+{
+  /* Index into the OPCODE element of the arc_opcode_hash_entry.  */
+  size_t index;
+
+  /* The specific ARC_OPCODE from the ARC_OPCODES table that was last
+     returned by this iterator.  */
+  const struct arc_opcode *opcode;
+};
+
 /* Forward declaration.  */
 static void assemble_insn
   (const struct arc_opcode *, const expressionS *, int,
@@ -577,6 +588,47 @@ arc_find_opcode (const char *name)
   return entry;
 }
 
+/* Initialise the iterator ITER.  */
+
+static void
+arc_opcode_hash_entry_iterator_init (struct arc_opcode_hash_entry_iterator *iter)
+{
+  iter->index = 0;
+  iter->opcode = NULL;
+}
+
+/* Return the next ARC_OPCODE from ENTRY, using ITER to hold state between
+   calls to this function.  Return NULL when all ARC_OPCODE entries have
+   been returned.  */
+
+static const struct arc_opcode *
+arc_opcode_hash_entry_iterator_next (const struct arc_opcode_hash_entry *entry,
+				     struct arc_opcode_hash_entry_iterator *iter)
+{
+  if (iter->opcode == NULL && iter->index == 0)
+    {
+      gas_assert (entry->count > 0);
+      iter->opcode = entry->opcode[iter->index];
+    }
+  else if (iter->opcode != NULL)
+    {
+      const char *old_name = iter->opcode->name;
+
+      iter->opcode++;
+      if ((iter->opcode - arc_opcodes >= (int) arc_num_opcodes)
+	  || (strcmp (old_name, iter->opcode->name) != 0))
+	{
+	  iter->index++;
+	  if (iter->index == entry->count)
+	    iter->opcode = NULL;
+	  else
+	    iter->opcode = entry->opcode[iter->index];
+	}
+    }
+
+  return iter->opcode;
+}
+
 /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
    is encoded as 'middle-endian' for a little-endian target.  FIXME!
    this function is used for regular 4 byte instructions as well.  */
@@ -1406,23 +1458,22 @@ find_opcode_match (const struct arc_opcode_hash_entry *entry,
 		   int nflgs,
 		   int *pcpumatch)
 {
-  const struct arc_opcode *first_opcode = entry->opcode[0];
-  const struct arc_opcode *opcode = first_opcode;
+  const struct arc_opcode *opcode;
+  struct arc_opcode_hash_entry_iterator iter;
   int ntok = *pntok;
   int got_cpu_match = 0;
   expressionS bktok[MAX_INSN_ARGS];
   int bkntok;
   expressionS emptyE;
 
-  gas_assert (entry->count > 0);
-  if (entry->count > 1)
-    as_fatal (_("unable to lookup `%s', too many opcode chains"),
-	      first_opcode->name);
+  arc_opcode_hash_entry_iterator_init (&iter);
   memset (&emptyE, 0, sizeof (emptyE));
   memcpy (bktok, tok, MAX_INSN_ARGS * sizeof (*tok));
   bkntok = ntok;
 
-  do
+  for (opcode = arc_opcode_hash_entry_iterator_next (entry, &iter);
+       opcode != NULL;
+       opcode = arc_opcode_hash_entry_iterator_next (entry, &iter))
     {
       const unsigned char *opidx;
       const unsigned char *flgidx;
@@ -1743,8 +1794,6 @@ find_opcode_match (const struct arc_opcode_hash_entry *entry,
       memcpy (tok, bktok, MAX_INSN_ARGS * sizeof (*tok));
       ntok = bkntok;
     }
-  while (++opcode - arc_opcodes < (int) arc_num_opcodes
-	 && !strcmp (opcode->name, first_opcode->name));
 
   if (*pcpumatch)
     *pcpumatch = got_cpu_match;
@@ -2054,9 +2103,8 @@ assemble_tokens (const char *opname,
     {
       const struct arc_opcode *opcode;
 
-      pr_debug ("%s:%d: assemble_tokens: %s trying opcode 0x%08X\n",
-		frag_now->fr_file, frag_now->fr_line, opcode->name,
-		opcode->opcode);
+      pr_debug ("%s:%d: assemble_tokens: %s\n",
+		frag_now->fr_file, frag_now->fr_line, opname);
       found_something = TRUE;
       opcode = find_opcode_match (entry, tok, &ntok, pflags,
 				  nflgs, &cpumatch);
diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
index f182318..69c65fc 100644
--- a/opcodes/arc-opc.c
+++ b/opcodes/arc-opc.c
@@ -1453,7 +1453,37 @@ const unsigned arc_NToperand = FKT_NT;
 
    The format of the opcode table is:
 
-   NAME OPCODE MASK CPU CLASS SUBCLASS { OPERANDS } { FLAGS }.  */
+   NAME OPCODE MASK CPU CLASS SUBCLASS { OPERANDS } { FLAGS }.
+
+   The table is organised such that, where possible, all instructions with
+   the same mnemonic are together in a block.  When the assembler searches
+   for a suitable instruction the entries are checked in table order, so
+   more specific, or specialised cases should appear earlier in the table.
+
+   As an example, consider two instructions 'add a,b,u6' and 'add
+   a,b,limm'.  The first takes a 6-bit immediate that is encoded within the
+   32-bit instruction, while the second takes a 32-bit immediate that is
+   encoded in a follow-on 32-bit, making the total instruction length
+   64-bits.  In this case the u6 variant must appear first in the table, as
+   all u6 immediates could also be encoded using the 'limm' extension,
+   however, we want to use the shorter instruction wherever possible.
+
+   It is possible though to split instructions with the same mnemonic into
+   multiple groups.  However, the instructions are still checked in table
+   order, even across groups.  The only time that instructions with the
+   same mnemonic should be split into different groups is when different
+   variants of the instruction appear in different architectures, in which
+   case, grouping all instructions from a particular architecture together
+   might be preferable to merging the instruction into the main instruction
+   table.
+
+   An example of this split instruction groups can be found with the 'sync'
+   instruction.  The core arc architecture provides a 'sync' instruction,
+   while the nps instruction set extension provides 'sync.rd' and
+   'sync.wr'.  The rd/wr flags are instruction flags, not part of the
+   mnemonic, so we end up with two groups for the sync instruction, the
+   first within the core arc instruction table, and the second within the
+   nps extension instructions.  */
 const struct arc_opcode arc_opcodes[] =
 {
 #include "arc-tbl.h"
-- 
2.5.1

RE: [PATCHv2 3/4] gas/arc: Handle multiple arc_opcode chains for same mnemonic

[Claudiu Zissulescu]

This is sane as well,
Claudiu

> -----Original Message-----
> From: Andrew Burgess [mailto:andrew.burgess@embecosm.com]
> Sent: Thursday, April 07, 2016 2:57 PM
> To: binutils@sourceware.org
> Cc: Claudiu.Zissulescu@synopsys.com; Cupertino.Miranda@synopsys.com;
> noamca@mellanox.com; Andrew Burgess
> Subject: [PATCHv2 3/4] gas/arc: Handle multiple arc_opcode chains for same
> mnemonic
> 
> This commit completes support for having multiple instructions with the
> same mnemonic in non-contiguous blocks within the arc_opcodes table.
> 
> The commit adds an iterator mechanism for the arc_opcode_hash_entry
> structure, which is then used in find_opcode_match to consider all
> arc_opcode entries with the same mnemonic, even when these instructions
> are stored in non-contiguous blocks.
> 
> I extend the comment on the arc_opcodes table to discuss how entries
> within the table are organised, and to mention how instructions can be
> split into multiple groups if needed, but that the table is still
> searched in table order.
> 
> There should be no user visible changes after this commit.
> 
> gas/ChangeLog:
> 
> 	* config/tc-arc.c (struct arc_opcode_hash_entry_iterator): New
> 	structure.
> 	(arc_opcode_hash_entry_iterator_init): New function.
> 	(arc_opcode_hash_entry_iterator_next): New function.
> 	(find_opcode_match): Iterate over all arc_opcode entries
> 	referenced by the arc_opcode_hash_entry passed in as a parameter.
> 
> opcodes/ChangeLog:
> 
> 	* arc-opc.c (arc_opcodes): Extend comment to discus table layout.
> ---
>  gas/ChangeLog       |  9 +++++++
>  gas/config/tc-arc.c | 72
> ++++++++++++++++++++++++++++++++++++++++++++---------
>  opcodes/ChangeLog   |  4 +++
>  opcodes/arc-opc.c   | 32 +++++++++++++++++++++++-
>  4 files changed, 104 insertions(+), 13 deletions(-)
> 
> diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
> index 5b76a07..7efc630 100644
> --- a/gas/config/tc-arc.c
> +++ b/gas/config/tc-arc.c
> @@ -318,6 +318,17 @@ struct arc_opcode_hash_entry
>    const struct arc_opcode **opcode;
>  };
> 
> +/* Structure used for iterating through an arc_opcode_hash_entry.  */
> +struct arc_opcode_hash_entry_iterator
> +{
> +  /* Index into the OPCODE element of the arc_opcode_hash_entry.  */
> +  size_t index;
> +
> +  /* The specific ARC_OPCODE from the ARC_OPCODES table that was last
> +     returned by this iterator.  */
> +  const struct arc_opcode *opcode;
> +};
> +
>  /* Forward declaration.  */
>  static void assemble_insn
>    (const struct arc_opcode *, const expressionS *, int,
> @@ -577,6 +588,47 @@ arc_find_opcode (const char *name)
>    return entry;
>  }
> 
> +/* Initialise the iterator ITER.  */
> +
> +static void
> +arc_opcode_hash_entry_iterator_init (struct
> arc_opcode_hash_entry_iterator *iter)
> +{
> +  iter->index = 0;
> +  iter->opcode = NULL;
> +}
> +
> +/* Return the next ARC_OPCODE from ENTRY, using ITER to hold state
> between
> +   calls to this function.  Return NULL when all ARC_OPCODE entries have
> +   been returned.  */
> +
> +static const struct arc_opcode *
> +arc_opcode_hash_entry_iterator_next (const struct
> arc_opcode_hash_entry *entry,
> +				     struct arc_opcode_hash_entry_iterator
> *iter)
> +{
> +  if (iter->opcode == NULL && iter->index == 0)
> +    {
> +      gas_assert (entry->count > 0);
> +      iter->opcode = entry->opcode[iter->index];
> +    }
> +  else if (iter->opcode != NULL)
> +    {
> +      const char *old_name = iter->opcode->name;
> +
> +      iter->opcode++;
> +      if ((iter->opcode - arc_opcodes >= (int) arc_num_opcodes)
> +	  || (strcmp (old_name, iter->opcode->name) != 0))
> +	{
> +	  iter->index++;
> +	  if (iter->index == entry->count)
> +	    iter->opcode = NULL;
> +	  else
> +	    iter->opcode = entry->opcode[iter->index];
> +	}
> +    }
> +
> +  return iter->opcode;
> +}
> +
>  /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
>     is encoded as 'middle-endian' for a little-endian target.  FIXME!
>     this function is used for regular 4 byte instructions as well.  */
> @@ -1406,23 +1458,22 @@ find_opcode_match (const struct
> arc_opcode_hash_entry *entry,
>  		   int nflgs,
>  		   int *pcpumatch)
>  {
> -  const struct arc_opcode *first_opcode = entry->opcode[0];
> -  const struct arc_opcode *opcode = first_opcode;
> +  const struct arc_opcode *opcode;
> +  struct arc_opcode_hash_entry_iterator iter;
>    int ntok = *pntok;
>    int got_cpu_match = 0;
>    expressionS bktok[MAX_INSN_ARGS];
>    int bkntok;
>    expressionS emptyE;
> 
> -  gas_assert (entry->count > 0);
> -  if (entry->count > 1)
> -    as_fatal (_("unable to lookup `%s', too many opcode chains"),
> -	      first_opcode->name);
> +  arc_opcode_hash_entry_iterator_init (&iter);
>    memset (&emptyE, 0, sizeof (emptyE));
>    memcpy (bktok, tok, MAX_INSN_ARGS * sizeof (*tok));
>    bkntok = ntok;
> 
> -  do
> +  for (opcode = arc_opcode_hash_entry_iterator_next (entry, &iter);
> +       opcode != NULL;
> +       opcode = arc_opcode_hash_entry_iterator_next (entry, &iter))
>      {
>        const unsigned char *opidx;
>        const unsigned char *flgidx;
> @@ -1743,8 +1794,6 @@ find_opcode_match (const struct
> arc_opcode_hash_entry *entry,
>        memcpy (tok, bktok, MAX_INSN_ARGS * sizeof (*tok));
>        ntok = bkntok;
>      }
> -  while (++opcode - arc_opcodes < (int) arc_num_opcodes
> -	 && !strcmp (opcode->name, first_opcode->name));
> 
>    if (*pcpumatch)
>      *pcpumatch = got_cpu_match;
> @@ -2054,9 +2103,8 @@ assemble_tokens (const char *opname,
>      {
>        const struct arc_opcode *opcode;
> 
> -      pr_debug ("%s:%d: assemble_tokens: %s trying opcode 0x%08X\n",
> -		frag_now->fr_file, frag_now->fr_line, opcode->name,
> -		opcode->opcode);
> +      pr_debug ("%s:%d: assemble_tokens: %s\n",
> +		frag_now->fr_file, frag_now->fr_line, opname);
>        found_something = TRUE;
>        opcode = find_opcode_match (entry, tok, &ntok, pflags,
>  				  nflgs, &cpumatch);
> diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
> index f182318..69c65fc 100644
> --- a/opcodes/arc-opc.c
> +++ b/opcodes/arc-opc.c
> @@ -1453,7 +1453,37 @@ const unsigned arc_NToperand = FKT_NT;
> 
>     The format of the opcode table is:
> 
> -   NAME OPCODE MASK CPU CLASS SUBCLASS { OPERANDS } { FLAGS }.  */
> +   NAME OPCODE MASK CPU CLASS SUBCLASS { OPERANDS } { FLAGS }.
> +
> +   The table is organised such that, where possible, all instructions with
> +   the same mnemonic are together in a block.  When the assembler
> searches
> +   for a suitable instruction the entries are checked in table order, so
> +   more specific, or specialised cases should appear earlier in the table.
> +
> +   As an example, consider two instructions 'add a,b,u6' and 'add
> +   a,b,limm'.  The first takes a 6-bit immediate that is encoded within the
> +   32-bit instruction, while the second takes a 32-bit immediate that is
> +   encoded in a follow-on 32-bit, making the total instruction length
> +   64-bits.  In this case the u6 variant must appear first in the table, as
> +   all u6 immediates could also be encoded using the 'limm' extension,
> +   however, we want to use the shorter instruction wherever possible.
> +
> +   It is possible though to split instructions with the same mnemonic into
> +   multiple groups.  However, the instructions are still checked in table
> +   order, even across groups.  The only time that instructions with the
> +   same mnemonic should be split into different groups is when different
> +   variants of the instruction appear in different architectures, in which
> +   case, grouping all instructions from a particular architecture together
> +   might be preferable to merging the instruction into the main instruction
> +   table.
> +
> +   An example of this split instruction groups can be found with the 'sync'
> +   instruction.  The core arc architecture provides a 'sync' instruction,
> +   while the nps instruction set extension provides 'sync.rd' and
> +   'sync.wr'.  The rd/wr flags are instruction flags, not part of the
> +   mnemonic, so we end up with two groups for the sync instruction, the
> +   first within the core arc instruction table, and the second within the
> +   nps extension instructions.  */
>  const struct arc_opcode arc_opcodes[] =
>  {
>  #include "arc-tbl.h"
> --
> 2.5.1

Re: [PATCHv2 3/4] gas/arc: Handle multiple arc_opcode chains for same mnemonic

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (struct arc_opcode_hash_entry_iterator): New
> 	structure.
> 	(arc_opcode_hash_entry_iterator_init): New function.
> 	(arc_opcode_hash_entry_iterator_next): New function.
> 	(find_opcode_match): Iterate over all arc_opcode entries
> 	referenced by the arc_opcode_hash_entry passed in as a parameter.
> 
> opcodes/ChangeLog:
> 
> 	* arc-opc.c (arc_opcodes): Extend comment to discus table layout.
 
Approved - please apply.

Cheers
  Nick

[PATCHv2 2/4] gas/arc: Additional work to support multiple arc_opcode chains

[Andrew Burgess]

Building on earlier commits, this commit moves along support for having
multiple arc_opcode entries in the arc_opcodes table that have the same
mnemonic (name) field, but are not stored in a contiguous block in the
table.

In this commit we support looking up the arc_opcode_hash_entry from the
hash table, and passing this along to the find_opcode_match function,
which then finds the specific arc_opcode that we're assembling.  We
still don't actually support the multiple chains of arc_opcode entries
in this commit, but the limitation is now isolated to the
find_opcode_match function.

There is no user visible change after this commit.

gas/ChangeLog:

	* config/tc-arc.c (arc_find_opcode): Now returns
	arc_opcode_hash_entry pointer.
	(find_opcode_match): Update argument type, extract arc_opcode from
	incoming arc_opcode_hash_entry.
	(find_special_case_pseudo): Update return type.
	(find_special_case_flag): Update return type.
	(find_special_case): Update return type.
	(assemble_tokens): Lookup arc_opcode_hash_entry based on
	instruction mnemonic, then use find_opcode_match to identify
	specific arc_opcode.
---
 gas/ChangeLog       | 13 ++++++++++
 gas/config/tc-arc.c | 68 ++++++++++++++++++++++++++---------------------------
 2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 1dcf125..5b76a07 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -564,27 +564,17 @@ static bfd_boolean assembling_insn = FALSE;
 
 /* Functions implementation.  */
 
-/* Return a pointer to the first entry in ARC_OPCODE_HASH that matches
-   NAME, or NULL if there are no matching entries.  */
+/* Return a pointer to ARC_OPCODE_HASH_ENTRY that identifies all
+   ARC_OPCODE entries in ARC_OPCODE_HASH that match NAME, or NULL if there
+   are no matching entries in ARC_OPCODE_HASH.  */
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 arc_find_opcode (const char *name)
 {
   const struct arc_opcode_hash_entry *entry;
-  const struct arc_opcode *opcode;
 
   entry = hash_find (arc_opcode_hash, name);
-  if (entry != NULL)
-    {
-      if (entry->count > 1)
-        as_fatal (_("unable to lookup `%s', too many opcode chains"),
-                  name);
-      opcode = *entry->opcode;
-    }
-  else
-    opcode = NULL;
-
-  return opcode;
+  return entry;
 }
 
 /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
@@ -1409,13 +1399,14 @@ check_cpu_feature (insn_subclass_t sc)
    syntax match.  */
 
 static const struct arc_opcode *
-find_opcode_match (const struct arc_opcode *first_opcode,
+find_opcode_match (const struct arc_opcode_hash_entry *entry,
 		   expressionS *tok,
 		   int *pntok,
 		   struct arc_flags *first_pflag,
 		   int nflgs,
 		   int *pcpumatch)
 {
+  const struct arc_opcode *first_opcode = entry->opcode[0];
   const struct arc_opcode *opcode = first_opcode;
   int ntok = *pntok;
   int got_cpu_match = 0;
@@ -1423,6 +1414,10 @@ find_opcode_match (const struct arc_opcode *first_opcode,
   int bkntok;
   expressionS emptyE;
 
+  gas_assert (entry->count > 0);
+  if (entry->count > 1)
+    as_fatal (_("unable to lookup `%s', too many opcode chains"),
+	      first_opcode->name);
   memset (&emptyE, 0, sizeof (emptyE));
   memcpy (bktok, tok, MAX_INSN_ARGS * sizeof (*tok));
   bkntok = ntok;
@@ -1873,7 +1868,7 @@ find_pseudo_insn (const char *opname,
 
 /* Assumes the expressionS *tok is of sufficient size.  */
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 find_special_case_pseudo (const char *opname,
 			  int *ntok,
 			  expressionS *tok,
@@ -1968,7 +1963,7 @@ find_special_case_pseudo (const char *opname,
   return arc_find_opcode (pseudo_insn->mnemonic_r);
 }
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 find_special_case_flag (const char *opname,
 			int *nflgs,
 			struct arc_flags *pflags)
@@ -1978,7 +1973,7 @@ find_special_case_flag (const char *opname,
   unsigned flag_idx, flag_arr_idx;
   size_t flaglen, oplen;
   const struct arc_flag_special *arc_flag_special_opcode;
-  const struct arc_opcode *opcode;
+  const struct arc_opcode_hash_entry *entry;
 
   /* Search for special case instruction.  */
   for (i = 0; i < arc_num_flag_special; i++)
@@ -2001,14 +1996,14 @@ find_special_case_flag (const char *opname,
 	  flaglen = strlen (flagnm);
 	  if (strcmp (opname + oplen, flagnm) == 0)
 	    {
-              opcode = arc_find_opcode (arc_flag_special_opcode->name);
+              entry = arc_find_opcode (arc_flag_special_opcode->name);
 
 	      if (*nflgs + 1 > MAX_INSN_FLGS)
 		break;
 	      memcpy (pflags[*nflgs].name, flagnm, flaglen);
 	      pflags[*nflgs].name[flaglen] = '\0';
 	      (*nflgs)++;
-	      return opcode;
+	      return entry;
 	    }
 	}
     }
@@ -2017,21 +2012,21 @@ find_special_case_flag (const char *opname,
 
 /* Used to find special case opcode.  */
 
-static const struct arc_opcode *
+static const struct arc_opcode_hash_entry *
 find_special_case (const char *opname,
 		   int *nflgs,
 		   struct arc_flags *pflags,
 		   expressionS *tok,
 		   int *ntok)
 {
-  const struct arc_opcode *opcode;
+  const struct arc_opcode_hash_entry *entry;
 
-  opcode = find_special_case_pseudo (opname, ntok, tok, nflgs, pflags);
+  entry = find_special_case_pseudo (opname, ntok, tok, nflgs, pflags);
 
-  if (opcode == NULL)
-    opcode = find_special_case_flag (opname, nflgs, pflags);
+  if (entry == NULL)
+    entry = find_special_case_flag (opname, nflgs, pflags);
 
-  return opcode;
+  return entry;
 }
 
 /* Given an opcode name, pre-tockenized set of argumenst and the
@@ -2045,27 +2040,30 @@ assemble_tokens (const char *opname,
 		 int nflgs)
 {
   bfd_boolean found_something = FALSE;
-  const struct arc_opcode *opcode;
+  const struct arc_opcode_hash_entry *entry;
   int cpumatch = 1;
 
   /* Search opcodes.  */
-  opcode = arc_find_opcode (opname);
+  entry = arc_find_opcode (opname);
 
   /* Couldn't find opcode conventional way, try special cases.  */
-  if (!opcode)
-    opcode = find_special_case (opname, &nflgs, pflags, tok, &ntok);
+  if (entry == NULL)
+    entry = find_special_case (opname, &nflgs, pflags, tok, &ntok);
 
-  if (opcode)
+  if (entry != NULL)
     {
+      const struct arc_opcode *opcode;
+
       pr_debug ("%s:%d: assemble_tokens: %s trying opcode 0x%08X\n",
 		frag_now->fr_file, frag_now->fr_line, opcode->name,
 		opcode->opcode);
-
       found_something = TRUE;
-      opcode = find_opcode_match (opcode, tok, &ntok, pflags, nflgs, &cpumatch);
-      if (opcode)
+      opcode = find_opcode_match (entry, tok, &ntok, pflags,
+				  nflgs, &cpumatch);
+      if (opcode != NULL)
 	{
 	  struct arc_insn insn;
+
 	  assemble_insn (opcode, tok, ntok, pflags, nflgs, &insn);
 	  emit_insn (&insn);
 	  return;
-- 
2.5.1

RE: [PATCHv2 2/4] gas/arc: Additional work to support multiple arc_opcode chains

[Claudiu Zissulescu]

This also seems all right to me.
Cheers,
Claudiu

> -----Original Message-----
> From: Andrew Burgess [mailto:andrew.burgess@embecosm.com]
> Sent: Thursday, April 07, 2016 2:57 PM
> To: binutils@sourceware.org
> Cc: Claudiu.Zissulescu@synopsys.com; Cupertino.Miranda@synopsys.com;
> noamca@mellanox.com; Andrew Burgess
> Subject: [PATCHv2 2/4] gas/arc: Additional work to support multiple
> arc_opcode chains
> 
> Building on earlier commits, this commit moves along support for having
> multiple arc_opcode entries in the arc_opcodes table that have the same
> mnemonic (name) field, but are not stored in a contiguous block in the
> table.
> 
> In this commit we support looking up the arc_opcode_hash_entry from the
> hash table, and passing this along to the find_opcode_match function,
> which then finds the specific arc_opcode that we're assembling.  We
> still don't actually support the multiple chains of arc_opcode entries
> in this commit, but the limitation is now isolated to the
> find_opcode_match function.
> 
> There is no user visible change after this commit.
> 
> gas/ChangeLog:
> 
> 	* config/tc-arc.c (arc_find_opcode): Now returns
> 	arc_opcode_hash_entry pointer.
> 	(find_opcode_match): Update argument type, extract arc_opcode
> from
> 	incoming arc_opcode_hash_entry.
> 	(find_special_case_pseudo): Update return type.
> 	(find_special_case_flag): Update return type.
> 	(find_special_case): Update return type.
> 	(assemble_tokens): Lookup arc_opcode_hash_entry based on
> 	instruction mnemonic, then use find_opcode_match to identify
> 	specific arc_opcode.
> ---
>  gas/ChangeLog       | 13 ++++++++++
>  gas/config/tc-arc.c | 68 ++++++++++++++++++++++++++-----------------------
> ----
>  2 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
> index 1dcf125..5b76a07 100644
> --- a/gas/config/tc-arc.c
> +++ b/gas/config/tc-arc.c
> @@ -564,27 +564,17 @@ static bfd_boolean assembling_insn = FALSE;
> 
>  /* Functions implementation.  */
> 
> -/* Return a pointer to the first entry in ARC_OPCODE_HASH that matches
> -   NAME, or NULL if there are no matching entries.  */
> +/* Return a pointer to ARC_OPCODE_HASH_ENTRY that identifies all
> +   ARC_OPCODE entries in ARC_OPCODE_HASH that match NAME, or NULL if
> there
> +   are no matching entries in ARC_OPCODE_HASH.  */
> 
> -static const struct arc_opcode *
> +static const struct arc_opcode_hash_entry *
>  arc_find_opcode (const char *name)
>  {
>    const struct arc_opcode_hash_entry *entry;
> -  const struct arc_opcode *opcode;
> 
>    entry = hash_find (arc_opcode_hash, name);
> -  if (entry != NULL)
> -    {
> -      if (entry->count > 1)
> -        as_fatal (_("unable to lookup `%s', too many opcode chains"),
> -                  name);
> -      opcode = *entry->opcode;
> -    }
> -  else
> -    opcode = NULL;
> -
> -  return opcode;
> +  return entry;
>  }
> 
>  /* Like md_number_to_chars but used for limms.  The 4-byte limm value,
> @@ -1409,13 +1399,14 @@ check_cpu_feature (insn_subclass_t sc)
>     syntax match.  */
> 
>  static const struct arc_opcode *
> -find_opcode_match (const struct arc_opcode *first_opcode,
> +find_opcode_match (const struct arc_opcode_hash_entry *entry,
>  		   expressionS *tok,
>  		   int *pntok,
>  		   struct arc_flags *first_pflag,
>  		   int nflgs,
>  		   int *pcpumatch)
>  {
> +  const struct arc_opcode *first_opcode = entry->opcode[0];
>    const struct arc_opcode *opcode = first_opcode;
>    int ntok = *pntok;
>    int got_cpu_match = 0;
> @@ -1423,6 +1414,10 @@ find_opcode_match (const struct arc_opcode
> *first_opcode,
>    int bkntok;
>    expressionS emptyE;
> 
> +  gas_assert (entry->count > 0);
> +  if (entry->count > 1)
> +    as_fatal (_("unable to lookup `%s', too many opcode chains"),
> +	      first_opcode->name);
>    memset (&emptyE, 0, sizeof (emptyE));
>    memcpy (bktok, tok, MAX_INSN_ARGS * sizeof (*tok));
>    bkntok = ntok;
> @@ -1873,7 +1868,7 @@ find_pseudo_insn (const char *opname,
> 
>  /* Assumes the expressionS *tok is of sufficient size.  */
> 
> -static const struct arc_opcode *
> +static const struct arc_opcode_hash_entry *
>  find_special_case_pseudo (const char *opname,
>  			  int *ntok,
>  			  expressionS *tok,
> @@ -1968,7 +1963,7 @@ find_special_case_pseudo (const char *opname,
>    return arc_find_opcode (pseudo_insn->mnemonic_r);
>  }
> 
> -static const struct arc_opcode *
> +static const struct arc_opcode_hash_entry *
>  find_special_case_flag (const char *opname,
>  			int *nflgs,
>  			struct arc_flags *pflags)
> @@ -1978,7 +1973,7 @@ find_special_case_flag (const char *opname,
>    unsigned flag_idx, flag_arr_idx;
>    size_t flaglen, oplen;
>    const struct arc_flag_special *arc_flag_special_opcode;
> -  const struct arc_opcode *opcode;
> +  const struct arc_opcode_hash_entry *entry;
> 
>    /* Search for special case instruction.  */
>    for (i = 0; i < arc_num_flag_special; i++)
> @@ -2001,14 +1996,14 @@ find_special_case_flag (const char *opname,
>  	  flaglen = strlen (flagnm);
>  	  if (strcmp (opname + oplen, flagnm) == 0)
>  	    {
> -              opcode = arc_find_opcode (arc_flag_special_opcode->name);
> +              entry = arc_find_opcode (arc_flag_special_opcode->name);
> 
>  	      if (*nflgs + 1 > MAX_INSN_FLGS)
>  		break;
>  	      memcpy (pflags[*nflgs].name, flagnm, flaglen);
>  	      pflags[*nflgs].name[flaglen] = '\0';
>  	      (*nflgs)++;
> -	      return opcode;
> +	      return entry;
>  	    }
>  	}
>      }
> @@ -2017,21 +2012,21 @@ find_special_case_flag (const char *opname,
> 
>  /* Used to find special case opcode.  */
> 
> -static const struct arc_opcode *
> +static const struct arc_opcode_hash_entry *
>  find_special_case (const char *opname,
>  		   int *nflgs,
>  		   struct arc_flags *pflags,
>  		   expressionS *tok,
>  		   int *ntok)
>  {
> -  const struct arc_opcode *opcode;
> +  const struct arc_opcode_hash_entry *entry;
> 
> -  opcode = find_special_case_pseudo (opname, ntok, tok, nflgs, pflags);
> +  entry = find_special_case_pseudo (opname, ntok, tok, nflgs, pflags);
> 
> -  if (opcode == NULL)
> -    opcode = find_special_case_flag (opname, nflgs, pflags);
> +  if (entry == NULL)
> +    entry = find_special_case_flag (opname, nflgs, pflags);
> 
> -  return opcode;
> +  return entry;
>  }
> 
>  /* Given an opcode name, pre-tockenized set of argumenst and the
> @@ -2045,27 +2040,30 @@ assemble_tokens (const char *opname,
>  		 int nflgs)
>  {
>    bfd_boolean found_something = FALSE;
> -  const struct arc_opcode *opcode;
> +  const struct arc_opcode_hash_entry *entry;
>    int cpumatch = 1;
> 
>    /* Search opcodes.  */
> -  opcode = arc_find_opcode (opname);
> +  entry = arc_find_opcode (opname);
> 
>    /* Couldn't find opcode conventional way, try special cases.  */
> -  if (!opcode)
> -    opcode = find_special_case (opname, &nflgs, pflags, tok, &ntok);
> +  if (entry == NULL)
> +    entry = find_special_case (opname, &nflgs, pflags, tok, &ntok);
> 
> -  if (opcode)
> +  if (entry != NULL)
>      {
> +      const struct arc_opcode *opcode;
> +
>        pr_debug ("%s:%d: assemble_tokens: %s trying opcode 0x%08X\n",
>  		frag_now->fr_file, frag_now->fr_line, opcode->name,
>  		opcode->opcode);
> -
>        found_something = TRUE;
> -      opcode = find_opcode_match (opcode, tok, &ntok, pflags, nflgs,
> &cpumatch);
> -      if (opcode)
> +      opcode = find_opcode_match (entry, tok, &ntok, pflags,
> +				  nflgs, &cpumatch);
> +      if (opcode != NULL)
>  	{
>  	  struct arc_insn insn;
> +
>  	  assemble_insn (opcode, tok, ntok, pflags, nflgs, &insn);
>  	  emit_insn (&insn);
>  	  return;
> --
> 2.5.1

Re: [PATCHv2 2/4] gas/arc: Additional work to support multiple arc_opcode chains

[Nick Clifton]

Hi Andrew,

> gas/ChangeLog:
> 
> 	* config/tc-arc.c (arc_find_opcode): Now returns
> 	arc_opcode_hash_entry pointer.
> 	(find_opcode_match): Update argument type, extract arc_opcode from
> 	incoming arc_opcode_hash_entry.
> 	(find_special_case_pseudo): Update return type.
> 	(find_special_case_flag): Update return type.
> 	(find_special_case): Update return type.
> 	(assemble_tokens): Lookup arc_opcode_hash_entry based on
> 	instruction mnemonic, then use find_opcode_match to identify
> 	specific arc_opcode.

Approved - please apply.

Cheers
  Nick

[PATCHv2 0/4] arc/nps: Add pipeline control instructions

[Andrew Burgess]

Second version of adding pipeline control instructions.  No
significant changes here, two clean up patches from round 1 that were
approved have been applied upstream, and this patch series rebased
onto current HEAD of master.

There's a new comment describing the layout of arc_opcodes table added
in patch #3, but everything else is unchanged.

Thanks,
Andrew

---

Andrew Burgess (4):
  gas/arc: Modify structure used to hold opcodes
  gas/arc: Additional work to support multiple arc_opcode chains
  gas/arc: Handle multiple arc_opcode chains for same mnemonic
  arc/nps400: Add new instructions

 gas/ChangeLog                    |  37 +++++++++
 gas/config/tc-arc.c              | 160 ++++++++++++++++++++++++++++++---------
 gas/config/tc-arc.h              |   2 +-
 gas/testsuite/gas/arc/nps400-2.d |  18 +++++
 gas/testsuite/gas/arc/nps400-2.s |  16 ++++
 opcodes/ChangeLog                |  10 +++
 opcodes/arc-nps400-tbl.h         |  17 +++++
 opcodes/arc-opc.c                |  82 +++++++++++++++++++-
 8 files changed, 306 insertions(+), 36 deletions(-)
 create mode 100644 gas/testsuite/gas/arc/nps400-2.d
 create mode 100644 gas/testsuite/gas/arc/nps400-2.s

-- 
2.5.1