Re: [PATCH] gas/arc: Allow --with-cpu configure option to change default cpu

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

* Nick Clifton <nickc@redhat.com> [2016-06-29 14:15:52 +0100]:

> Hi Andrew,
> 
> > I've attached a new revision, this includes NEWS entry, and the help
> > text that makes it clear the option is only supported on ARC.
> 
> Sorry - this version still has a few problems:

No problem.  Thanks for your patience with the review.

> 
>   1.  If you configure a toolchain as "--target=arc-elf", but do
>       not provide a --with-cpu= option, then the assembler fails
>       to run, producing this error:
> 
>         Assembler messages:
>         Fatal error: unknown architecture: 
> 
>      This is because the config script sets TARGET_WITH_CPU to be "".
> 
> 
>   (In the code fragments below I have assumed that this has been corrected
>    so that TARGET_WITH_CPU is not defined unless --with-cpu is
>    used).

Fixed by moving the definition of TARGET_WITH_CPU into the AC_ARG_WITH
in the configure.ac script.

> 
> 
>   2. The patch does not provide a way for the user to find out the default
>      cpu type.  Ideally this should be included in the --help output, like this:
> 
> @@ -3401,7 +3405,7 @@ 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, "  -mcpu=<cpu name>\t  assemble for CPU <cpu name> (default: %s)\n", TARGET_WITH_CPU);
> 
> 
>       Additionally, you might like to extend the text displayed with --version
>       so that it includes the cpu type as well.  Like this:
> 
> diff --git a/gas/as.c b/gas/as.c
> index 8784fb4..90f18c0 100644
> --- a/gas/as.c
> +++ b/gas/as.c
> @@ -665,8 +665,13 @@ parse_args (int * pargc, char *** pargv)
>  This program is free software; you may redistribute it under the terms of\n\
>  the GNU General Public License version 3 or later.\n\
>  This program has absolutely no warranty.\n"));
> +#ifdef TARGET_WITH_CPU
> +         printf (_("This assembler was configured for a target of `%s' and default cpu type `%s'\n"),
> +                 TARGET_ALIAS, TARGET_WITH_CPU);
> +#else
>           printf (_("This assembler was configured for a target of `%s'.\n"),
>                   TARGET_ALIAS);
> +#endif
>           exit (EXIT_SUCCESS);
>  
>         case OPTION_EMULATION:

Both changes applied.

>   3. Please could you change:
> 
> > +dnl Option --with-cpu=TYPE allows configure type control of the default
> > +dnl cpu type within the assembler.  Not all targets support this option
> > +dnl yet, which is why there is no help string, check the code to see if
> > +dnl your target supports this option.
> > +AC_ARG_WITH(cpu,
> > +            AS_HELP_STRING([--with-cpu=CPU],
> > +            [default cpu variant is CPU (only supported on ARC)]),
> > +            [],[])
> 
> to:
> 
>   +dnl Option --with-cpu=TYPE allows configure type control of the default
>   +dnl cpu type within the assembler.  Currently only the ARC target
>   +dnl supports this feature, but others may be added in the future.
>   +AC_ARG_WITH(cpu,
>   +            AS_HELP_STRING([--with-cpu=CPU],
>   +            [default cpu variant is CPU (currently only supported on ARC)]),
>   +            [],[])

Done.

>   4. Optional: There is no error checking of with --with-cpu value.  So I can
>      configure with --with-cpu=fred and not get an error until I try to run the
>      assembler.  Fixing this might not actually be a good idea since it will 
>      involve duplicating the list of acceptable cpu names, meaning that there
>      would then be two lists that need to be kept in sync.

I did think about this, and GCC does do similar checking, but as you
predict the list in GCC is out of date.  Given that an incorrect
default should be spotted fairly quickly I figured that not checking
at configure time would be best.

I'm happy to add the check though, if you think it's for the best, but
from reading your comment it sounds like you're in two minds about it
too.

Here's a new version of the patch, without item #4.  Let me know what
you think.

Thanks,
Andrew

---

gas/arc: Allow --with-cpu configure option to change default cpu

Add a new configure time option --with-cpu=CPU that can be used to
change the default cpu selection.  Currently this is only supported on
ARC.

If the user specifies --with-cpu=FOO at configure time then the constant
TARGET_WITH_CPU will be defined to "FOO" in config.h, the
TARGET_WITH_CPU constant can then be used in the config/tc-*.c code as a
default string to select an appropriate cpu if the user does not
explicitly select a cpu at gas run-time.

gas/ChangeLog:

	* config.in (TARGET_WITH_CPU): Undefine.
	* configure.ac: Add --with-cpu support, and define in config.h.
	* configure: Regenerate.
	* config/tc-arc.c: Ensure TARGET_WITH_CPU is defined.
	(md_begin): Use TARGET_WITH_CPU to select default CPU.
	(md_show_usage): Display default cpu type.
	* as.c (parse_args): Show default cpu type if it's defined.
	* NEWS: Mention new configure option.
---
 gas/ChangeLog       | 11 +++++++++++
 gas/NEWS            |  3 +++
 gas/as.c            |  6 ++++++
 gas/config.in       |  3 +++
 gas/config/tc-arc.c |  9 +++++++--
 gas/configure       | 18 ++++++++++++++++--
 gas/configure.ac    | 11 +++++++++++
 7 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/gas/NEWS b/gas/NEWS
index f5708a2..4bcf6de 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -34,6 +34,9 @@
 
 * Add assembly-time relaxation option for ARC cpus.
 
+* Add --with-cpu=TYPE configure option for ARC gas.  This allows the default
+  cpu type to be adjusted at configure time.
+
 Changes in 2.26:
 
 * Add a configure option --enable-compressed-debug-sections={all,gas} to
diff --git a/gas/as.c b/gas/as.c
index 8784fb4..6ccf167 100644
--- a/gas/as.c
+++ b/gas/as.c
@@ -665,8 +665,14 @@ parse_args (int * pargc, char *** pargv)
 This program is free software; you may redistribute it under the terms of\n\
 the GNU General Public License version 3 or later.\n\
 This program has absolutely no warranty.\n"));
+#ifdef TARGET_WITH_CPU
+	  printf (_("This assembler was configured for a target of `%s' "
+		    "and default,\ncpu type `%s'.\n"),
+		  TARGET_ALIAS, TARGET_WITH_CPU);
+#else
 	  printf (_("This assembler was configured for a target of `%s'.\n"),
 		  TARGET_ALIAS);
+#endif
 	  exit (EXIT_SUCCESS);
 
 	case OPTION_EMULATION:
diff --git a/gas/config.in b/gas/config.in
index e06f160..5129c28 100644
--- a/gas/config.in
+++ b/gas/config.in
@@ -318,6 +318,9 @@
 /* Target vendor. */
 #undef TARGET_VENDOR
 
+/* Target specific CPU. */
+#undef TARGET_WITH_CPU
+
 /* Use b modifier when opening binary files? */
 #undef USE_BINARY_FOPEN
 
diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 155d991..2046604 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -51,6 +51,10 @@
 /* Equal to MAX_PRECISION in atof-ieee.c.  */
 #define MAX_LITTLENUMS 6
 
+#ifndef TARGET_WITH_CPU
+#define TARGET_WITH_CPU "arc700"
+#endif /* TARGET_WITH_CPU */
+
 /* Enum used to enumerate the relaxable ins operands.  */
 enum rlx_operand_type
 {
@@ -2466,7 +2470,7 @@ md_begin (void)
   const struct arc_opcode *opcode = arc_opcodes;
 
   if (!mach_type_specified_p)
-    arc_select_cpu ("arc700");
+    arc_select_cpu (TARGET_WITH_CPU);
 
   /* The endianness can be chosen "at the factory".  */
   target_big_endian = byte_order == BIG_ENDIAN;
@@ -3401,7 +3405,8 @@ 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, "  -mcpu=<cpu name>\t  assemble for CPU <cpu name> "
+           "(default: %s)\n", TARGET_WITH_CPU);
   fprintf (stream, "  -mcpu=nps400\t\t  same as -mcpu=arc700 -mnps400\n");
   fprintf (stream, "  -mA6/-mARC600/-mARC601  same as -mcpu=arc600\n");
   fprintf (stream, "  -mA7/-mARC700\t\t  same as -mcpu=arc700\n");
diff --git a/gas/configure b/gas/configure
index b6298b5..b66f712 100755
--- a/gas/configure
+++ b/gas/configure
@@ -771,6 +771,7 @@ enable_x86_relax_relocations
 enable_elf_stt_common
 enable_werror
 enable_build_warnings
+with_cpu
 enable_nls
 enable_maintainer_mode
 with_system_zlib
@@ -1435,6 +1436,8 @@ Optional Packages:
   --with-pic              try to use only PIC/non-PIC objects [default=use
                           both]
   --with-gnu-ld           assume the C compiler uses GNU ld [default=no]
+  --with-cpu=CPU          default cpu variant is CPU (currently only supported
+                          on ARC)
   --with-system-zlib      use installed libz
 
 Some influential environment variables:
@@ -10982,7 +10985,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 10985 "configure"
+#line 10988 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11088,7 +11091,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11091 "configure"
+#line 11094 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11828,6 +11831,17 @@ fi
 ac_config_headers="$ac_config_headers config.h:config.in"
 
 
+
+# Check whether --with-cpu was given.
+if test "${with_cpu+set}" = set; then :
+  withval=$with_cpu;
+cat >>confdefs.h <<_ACEOF
+#define TARGET_WITH_CPU "${with_cpu}"
+_ACEOF
+
+fi
+
+
 # PR 14072
 
 
diff --git a/gas/configure.ac b/gas/configure.ac
index 8f71825..13f5107 100644
--- a/gas/configure.ac
+++ b/gas/configure.ac
@@ -106,6 +106,17 @@ AM_BINUTILS_WARNINGS
 # Generate a header file
 AC_CONFIG_HEADERS(config.h:config.in)
 
+dnl Option --with-cpu=TYPE allows configure type control of the default
+dnl cpu type within the assembler.  Currently only the ARC target
+dnl supports this feature, but others may be added in the future.
+AC_ARG_WITH(cpu,
+            AS_HELP_STRING([--with-cpu=CPU],
+            [default cpu variant is CPU (currently only supported on ARC)]),
+            [AC_DEFINE_UNQUOTED(TARGET_WITH_CPU,
+                                "${with_cpu}",
+                                [Target specific CPU.])],
+            [])
+
 # PR 14072
 AH_VERBATIM([00_CONFIG_H_CHECK],
 [/* Check that config.h is #included before system headers
-- 
2.5.1