Re: [PATCH 0/2] [PUSHED/OBV] gas/arc: Add nps400 support to .cpu directive

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Nick Clifton <nickc@redhat.com> [2016-04-18 12:07:44 +0100]:

> Hi Andrew,
> 
> > I have taken the liberty of leaving this commit in the tree.  I'll
> > keep my fingers crossed that a global maintainer doesn't object :-)
> 
> Actually I do object, sorry. :-{
> 
> There are problems with both of the patches:
> 
> [PATCH 1/2] gas/arc: Support NPS400 in .cpu directive
> 
>   The problem here is not the patch itself, it is OK, but rather
>   that it does not go far enough.  The patch adds support for the
>   NPS400 to the .cpu directive, but it does not update the 
>   documentation in gas/doc/c-arc.texi detailing the acceptable
>   arguments to the .cpu directive.
> 
>   [A patch to update the documentation as indicated is pre-approved].
> 
>   Further I would suggest that this code can be improved.  Since
>   there already is a table of known ARC architecture variants in tc-arc.c
>   (cpu_types[], line 420), it would make sense to change the code in
>   arc_option() to scan this table.  That way when a new architecture is
>   added in the future the programmer will not have to remember to 
>   update the arc_option() function as well.
> 
>   Additionally the md_show_usage() function could be extended to list
>   exactly which architectures are supported by the -mcpu= command line
>   option; again by scanning the cpu_types table.  Other targets already
>   do this sort of thing.
> 
>   [The patch can stay in, but please do consider the above advice].

Below is a patch that does the things you mention above:

  - Uses the same table to drive -mcpu and .cpu
  - Uses the same table to generate the cpu list in the docs.

OK?

Thanks,
Andrew

---

arc/gas: Unify -mcpu option and .cpu directive handling

This change makes use of the single cpu_types table for handling the
.cpu directive and the -mcpu command line option.  At the same time the
help text for -mcpu is extended to include the list of possible cpu
types, generated from the table.

gas/ChangeLog:

	* config/tc-arc.c (MAX_NUMBER_OF_ALIASES): Define.
	(struct cpu_type): Add aliases member, convert string points to
	NULL instead of 0.
	(cpu_types): Fill in the aliases field.
	(arc_handle_cpu_option_string): New function.
	(arc_option): Use arc_handle_cpu_option_string to process the
	option.
	(arc_show_cpu_list): New function.
	(md_show_usage): Call arc_show_cpu_list.  Convert help text to
	lower case to match all other help text.  Remove use of tabs,
	inline with global help text.
---
 gas/ChangeLog       |  14 ++++++
 gas/config/tc-arc.c | 123 +++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 97 insertions(+), 40 deletions(-)

diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 169b05c..c556f54 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -408,10 +408,14 @@ static struct hash_control *arc_reg_hash;
 /* The hash table of aux register symbols.  */
 static struct hash_control *arc_aux_hash;
 
+/* Maximum number of CPU aliases in the cpu_type table.  */
+#define MAX_NUMBER_OF_ALIASES 2
+
 /* A table of CPU names and opcode sets.  */
 static const struct cpu_type
 {
   const char *name;
+  const char *alises [MAX_NUMBER_OF_ALIASES + 1];
   unsigned flags;
   int mach;
   unsigned eflags;
@@ -419,17 +423,18 @@ static const struct cpu_type
 }
   cpu_types[] =
 {
-  { "arc600", ARC_OPCODE_ARC600,  bfd_mach_arc_arc600,
-    E_ARC_MACH_ARC600,  0x00},
-  { "arc700", ARC_OPCODE_ARC700,  bfd_mach_arc_arc700,
-    E_ARC_MACH_ARC700,  0x00},
-  { "nps400", ARC_OPCODE_ARC700 | ARC_OPCODE_NPS400, bfd_mach_arc_nps400,
+  { "ARC600", { "ARC601", "A6", NULL },
+    ARC_OPCODE_ARC600,  bfd_mach_arc_arc600, E_ARC_MACH_ARC600,  0x00},
+  { "ARC700", { "A7", NULL },
+    ARC_OPCODE_ARC700,  bfd_mach_arc_arc700, E_ARC_MACH_ARC700,  0x00},
+  { "NPS400", { NULL },
+    ARC_OPCODE_ARC700 | ARC_OPCODE_NPS400, bfd_mach_arc_nps400,
     E_ARC_MACH_NPS400,  0x00},
-  { "arcem",  ARC_OPCODE_ARCv2EM, bfd_mach_arc_arcv2,
-    EF_ARC_CPU_ARCV2EM, ARC_CD},
-  { "archs",  ARC_OPCODE_ARCv2HS, bfd_mach_arc_arcv2,
-    EF_ARC_CPU_ARCV2HS, ARC_CD},
-  { 0, 0, 0, 0, 0 }
+  { "ARCEM", { "EM", NULL },
+    ARC_OPCODE_ARCv2EM, bfd_mach_arc_arcv2, EF_ARC_CPU_ARCV2EM, ARC_CD},
+  { "ARCHS", { "HS", NULL },
+    ARC_OPCODE_ARCv2HS, bfd_mach_arc_arcv2, EF_ARC_CPU_ARCV2HS, ARC_CD},
+  { NULL, { NULL }, 0, 0, 0, 0 }
 };
 
 /* Used by the arc_reloc_op table.  Order is important.  */
@@ -861,6 +866,40 @@ arc_lcomm (int ignore)
     symbol_get_bfdsym (symbolP)->flags |= BSF_OBJECT;
 }
 
+/* Take the string passed to the .cpu directive and configure the assembler
+   as though the -mcpu option was used.  Raises an error if the string is
+   not a valid cpu name.  */
+
+static void
+arc_handle_cpu_option_string (const char *cpu)
+{
+  int i;
+
+  for (i = 0; cpu_types[i].name != NULL; ++i)
+    {
+      if (!strcmp (cpu_types[i].name, cpu))
+        {
+          md_parse_option (OPTION_MCPU, cpu_types[i].name);
+          return;
+        }
+      else
+        {
+          int j;
+
+          for (j = 0; cpu_types[i].alises[j] != NULL; ++j)
+            {
+              if (!strcmp (cpu_types[i].alises[j], cpu))
+                {
+                  md_parse_option (OPTION_MCPU, cpu_types[i].name);
+                  return;
+                }
+            }
+        }
+    }
+
+  as_fatal (_("could not find the architecture"));
+}
+
 /* Select the cpu we're assembling for.  */
 
 static void
@@ -878,31 +917,7 @@ arc_option (int ignore ATTRIBUTE_UNUSED)
 
   if (!mach_type_specified_p)
     {
-      if ((!strcmp ("ARC600", cpu))
-	  || (!strcmp ("ARC601", cpu))
-	  || (!strcmp ("A6", cpu)))
-	{
-	  md_parse_option (OPTION_MCPU, "arc600");
-	}
-      else if ((!strcmp ("ARC700", cpu))
-	       || (!strcmp ("A7", cpu)))
-	{
-	  md_parse_option (OPTION_MCPU, "arc700");
-	}
-      else if (!strcmp ("EM", cpu))
-	{
-	  md_parse_option (OPTION_MCPU, "arcem");
-	}
-      else if (!strcmp ("HS", cpu))
-	{
-	  md_parse_option (OPTION_MCPU, "archs");
-	}
-      else if (!strcmp ("NPS400", cpu))
-	{
-	  md_parse_option (OPTION_MCPU, "nps400");
-	}
-      else
-	as_fatal (_("could not find the architecture"));
+      arc_handle_cpu_option_string (cpu);
 
       if (!bfd_set_arch_mach (stdoutput, bfd_arch_arc, mach))
 	as_fatal (_("could not set architecture and machine"));
@@ -3238,21 +3253,49 @@ md_parse_option (int c, const char *arg ATTRIBUTE_UNUSED)
   return 1;
 }
 
+/* Display the list of cpu names for use in the help text.  */
+static void
+arc_show_cpu_list (FILE *stream)
+{
+  int i, offset;
+  static const char *spaces = "                          ";
+
+  fprintf (stream, spaces);
+  offset = strlen (spaces);
+  for (i = 0; cpu_types[i].name != NULL; ++i)
+    {
+      bfd_boolean last = (cpu_types[i + 1].name == NULL);
+
+      /* If displaying the new cpu name string, and the ', ' (for all but
+         the last one) will take us past a target width of 80 characters,
+         then it's time for a new line.  */
+      if (offset + strlen (cpu_types[i].name) + (last ? 0 : 2) > 80)
+        {
+          fprintf (stream, "\n%s", spaces);
+          offset = strlen (spaces);
+        }
+
+      fprintf (stream, "%s%s", cpu_types[i].name, (last ? "\n" : ", "));
+      offset += strlen (cpu_types [i].name) + (last ? 0 : 2);
+    }
+}
+
 void
 md_show_usage (FILE *stream)
 {
   fprintf (stream, _("ARC-specific assembler options:\n"));
 
-  fprintf (stream, "  -mcpu=<cpu name>\t  assemble for CPU <cpu name>\n");
-  fprintf (stream,
-	   "  -mcode-density\t  enable code density option for ARC EM\n");
-
+  fprintf (stream, "\
+  -mcpu=<cpu name>        assemble for CPU <cpu name>, one of:\n");
+  arc_show_cpu_list (stream);
+  fprintf (stream, "\
+  -mcode-density          enable code density option for ARC EM\n");
   fprintf (stream, _("\
   -EB                     assemble code for a big-endian cpu\n"));
   fprintf (stream, _("\
   -EL                     assemble code for a little-endian cpu\n"));
   fprintf (stream, _("\
-  -mrelax                  Enable relaxation\n"));
+  -mrelax                 enable relaxation\n"));
 
 }
 
-- 
2.5.1