Re: [PATCH] Fix and extend features of .cpu directive.

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

* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-11-17 14:59:47 +0100]:

> In the below commit, the ARC .cpu directive implementation was
> refurbished.
> 
> commit bb65a718b601ecfebd1ebe5be71728d5c359c31f
> Author: Andrew Burgess <andrew.burgess@embecosm.com>
> 
>     gas/arc: Don't rely on bfd list of cpu type for cpu selection
> 
> Unfortunately, a small error was introduced, namely, when an option is
> given using command line, it may be overwritten/deleted when
> interpreting the .cpu pseudo-op, thus, leading to impossibility of
> building toolchains for cpus like ARCEM.
> 
> This patch solves the above issue, and extends the way how we check
> command line options (i.e., features) versus chosen cpu configuration,
> emitting errors if one request a feature which is not present in a
> specific architecture.
> 
> OK to apply?

This patch looks fine to me.  Sorry for introducing this issue.

I'm not a reviewer though, so you'll still need an official review.

Thanks,
Andrew



> Claudiu
> 
> 
> gas/
> 2016-11-17  Claudiu Zissulescu  <claziss@synopsys.com>
> 
> 	* testsuite/gas/arc/cl-warn.s: New file.
> 	* testsuite/gas/arc/cpu-pseudop-1.d: Likewise.
> 	* testsuite/gas/arc/cpu-pseudop-1.s: Likewise.
> 	* testsuite/gas/arc/cpu-pseudop-2.d: Likewise.
> 	* testsuite/gas/arc/cpu-pseudop-2.s: Likewise.
> 	* testsuite/gas/arc/cpu-warn2.s: Likewise.
> 	* config/tc-arc.c (selected_cpu): Initialize.
> 	(feature_type): New struct.
> 	(feature_list): New variable.
> 	(arc_check_feature): New function.
> 	(arc_select_cpu): Check for .cpu duplicates. Don't overwrite the
> 	current cpu features. Check if a feature is available for a given
> 	cpu.
> 	(md_parse_option): Test if features are available for a given cpu.
> ---
>  gas/config/tc-arc.c                   | 77 +++++++++++++++++++++++++++--------
>  gas/testsuite/gas/arc/cl-warn.s       |  5 +++
>  gas/testsuite/gas/arc/cpu-pseudop-1.d | 12 ++++++
>  gas/testsuite/gas/arc/cpu-pseudop-1.s |  6 +++
>  gas/testsuite/gas/arc/cpu-pseudop-2.d | 11 +++++
>  gas/testsuite/gas/arc/cpu-pseudop-2.s |  5 +++
>  gas/testsuite/gas/arc/cpu-warn2.s     |  4 ++
>  7 files changed, 102 insertions(+), 18 deletions(-)
>  create mode 100644 gas/testsuite/gas/arc/cl-warn.s
>  create mode 100644 gas/testsuite/gas/arc/cpu-pseudop-1.d
>  create mode 100644 gas/testsuite/gas/arc/cpu-pseudop-1.s
>  create mode 100644 gas/testsuite/gas/arc/cpu-pseudop-2.d
>  create mode 100644 gas/testsuite/gas/arc/cpu-pseudop-2.s
>  create mode 100644 gas/testsuite/gas/arc/cpu-warn2.s
> 
> diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
> index 06aee48..376ac43 100644
> --- a/gas/config/tc-arc.c
> +++ b/gas/config/tc-arc.c
> @@ -451,7 +451,23 @@ static const struct cpu_type
>  };
>  
>  /* Information about the cpu/variant we're assembling for.  */
> -static struct cpu_type selected_cpu;
> +static struct cpu_type selected_cpu = { 0, 0, 0, 0, 0 };
> +
> +/* A table with options.  */
> +static const struct feature_type
> +{
> +  unsigned feature;
> +  unsigned cpus;
> +  const char *name;
> +}
> +  feature_list[] =
> +{
> +  { ARC_CD, ARC_OPCODE_ARCV2, "code-density" },
> +  { ARC_NPS400, ARC_OPCODE_ARC700, "nps400" },
> +  { ARC_SPFP, ARC_OPCODE_ARCFPX, "single-precision FPX" },
> +  { ARC_DPFP, ARC_OPCODE_ARCFPX, "double-precision FPX" },
> +  { ARC_FPUDA, ARC_OPCODE_ARCv2EM, "double assist FP" }
> +};
>  
>  /* Used by the arc_reloc_op table.  Order is important.  */
>  #define O_gotoff  O_md1     /* @gotoff relocation.  */
> @@ -775,6 +791,27 @@ md_number_to_chars_midend (char *buf, unsigned long long val, int n)
>      }
>  }
>  
> +/* Check if a feature is allowed for a specific CPU.  */
> +
> +static void
> +arc_check_feature (void)
> +{
> +  unsigned i;
> +
> +  if (!selected_cpu.features
> +      || !selected_cpu.name)
> +    return;
> +  for (i = 0; (i < ARRAY_SIZE (feature_list)); i++)
> +    {
> +      if ((selected_cpu.features & feature_list[i].feature)
> +	  && !(selected_cpu.flags & feature_list[i].cpus))
> +	{
> +	  as_bad (_("invalid %s option for %s cpu"), feature_list[i].name,
> +		  selected_cpu.name);
> +	}
> +    }
> +}
> +
>  /* Select an appropriate entry from CPU_TYPES based on ARG and initialise
>     the relevant static global variables.  Parameter SEL describes where
>     this selection originated from.  */
> @@ -790,6 +827,10 @@ arc_select_cpu (const char *arg, enum mach_selection_type sel)
>    gas_assert (sel != MACH_SELECTION_FROM_DEFAULT
>                || mach_selection_mode == MACH_SELECTION_NONE);
>  
> +  if ((mach_selection_mode == MACH_SELECTION_FROM_CPU_DIRECTIVE)
> +      && (sel == MACH_SELECTION_FROM_CPU_DIRECTIVE))
> +    as_bad (_("Multiple .cpu directives found"));
> +
>    /* Look for a matching entry in CPU_TYPES array.  */
>    for (i = 0; cpu_types[i].name; ++i)
>      {
> @@ -807,22 +848,25 @@ arc_select_cpu (const char *arg, enum mach_selection_type sel)
>                    && selected_cpu.mach != cpu_types[i].mach)
>                  {
>                    as_warn (_("Command-line value overrides \".cpu\" directive"));
> -                  return;
>                  }
> +	      return;
>              }
>  
> -          /* Initialise static global data about selected machine type.  */
> -          selected_cpu.flags = cpu_types[i].flags;
> -          selected_cpu.name = cpu_types[i].name;
> -          selected_cpu.features = cpu_types[i].features;
> -          selected_cpu.mach = cpu_types[i].mach;
> -          cpu_flags = cpu_types[i].eflags;
> +	  /* Initialise static global data about selected machine type.  */
> +	  selected_cpu.flags = cpu_types[i].flags;
> +	  selected_cpu.name = cpu_types[i].name;
> +	  selected_cpu.features |= cpu_types[i].features;
> +	  selected_cpu.mach = cpu_types[i].mach;
> +	  cpu_flags = cpu_types[i].eflags;
>            break;
>          }
>      }
>  
>    if (!cpu_types[i].name)
>      as_fatal (_("unknown architecture: %s\n"), arg);
> +
> +  /* Check if set features are compatible with the chosen CPU.  */
> +  arc_check_feature ();
>    gas_assert (cpu_flags != 0);
>    selected_cpu.eflags = (arc_initial_eflag & ~EF_ARC_MACH_MSK) | cpu_flags;
>    mach_selection_mode = sel;
> @@ -3304,11 +3348,8 @@ md_parse_option (int c, const char *arg ATTRIBUTE_UNUSED)
>        break;
>  
>      case OPTION_CD:
> -      /* This option has an effect only on ARC EM.  */
> -      if (selected_cpu.flags & ARC_OPCODE_ARCv2EM)
> -	selected_cpu.features |= ARC_CD;
> -      else
> -	as_warn (_("Code density option invalid for selected CPU"));
> +      selected_cpu.features |= ARC_CD;
> +      arc_check_feature ();
>        break;
>  
>      case OPTION_RELAX:
> @@ -3317,22 +3358,22 @@ md_parse_option (int c, const char *arg ATTRIBUTE_UNUSED)
>  
>      case OPTION_NPS400:
>        selected_cpu.features |= ARC_NPS400;
> +      arc_check_feature ();
>        break;
>  
>      case OPTION_SPFP:
>        selected_cpu.features |= ARC_SPFP;
> +      arc_check_feature ();
>        break;
>  
>      case OPTION_DPFP:
>        selected_cpu.features |= ARC_DPFP;
> +      arc_check_feature ();
>        break;
>  
>      case OPTION_FPUDA:
> -      /* This option has an effect only on ARC EM.  */
> -      if (selected_cpu.flags & ARC_OPCODE_ARCv2EM)
> -	selected_cpu.features |= ARC_FPUDA;
> -      else
> -	as_warn (_("FPUDA invalid for selected CPU"));
> +      selected_cpu.features |= ARC_FPUDA;
> +      arc_check_feature ();
>        break;
>  
>      /* Dummy options are accepted but have no effect.  */
> diff --git a/gas/testsuite/gas/arc/cl-warn.s b/gas/testsuite/gas/arc/cl-warn.s
> new file mode 100644
> index 0000000..63199cf
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/cl-warn.s
> @@ -0,0 +1,5 @@
> +; Test command line option compatibility checking.
> +; { dg-do assemble }
> +; { dg-options "--mcpu=archs -mdpfp" }
> +; { dg-error ".* invalid double-precision FPX option for archs cpu" "" { target arc*-*-* } 0 }
> +	nop
> diff --git a/gas/testsuite/gas/arc/cpu-pseudop-1.d b/gas/testsuite/gas/arc/cpu-pseudop-1.d
> new file mode 100644
> index 0000000..09c47c9
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/cpu-pseudop-1.d
> @@ -0,0 +1,12 @@
> +#as: -mcpu=arcem -mcode-density -mdpfp
> +#objdump: -dp -M dpfp
> +
> +.*: +file format .*arc.*
> +private flags = 0x305: -mcpu=ARCv2EM .*
> +
> +
> +Disassembly of section .text:
> +
> +00000000 <.text>:
> +   0:	4af7                	sub_s	r15,r2,r15
> +   2:	3211 00c1           	dsubh12	r1,r2,r3
> diff --git a/gas/testsuite/gas/arc/cpu-pseudop-1.s b/gas/testsuite/gas/arc/cpu-pseudop-1.s
> new file mode 100644
> index 0000000..40217aa
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/cpu-pseudop-1.s
> @@ -0,0 +1,6 @@
> +;;; Check if user can use additional command line options.
> +	.cpu EM
> +	.section	.text
> +
> +	sub_s r15,r2,r15	;code density instruction
> +	dsubh12 r1,r2,r3	;double-precision instruction
> diff --git a/gas/testsuite/gas/arc/cpu-pseudop-2.d b/gas/testsuite/gas/arc/cpu-pseudop-2.d
> new file mode 100644
> index 0000000..3bde329
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/cpu-pseudop-2.d
> @@ -0,0 +1,11 @@
> +#as: -mcpu=archs
> +#objdump: -dp
> +
> +.*: +file format .*arc.*
> +private flags = 0x306: -mcpu=ARCv2HS .*
> +
> +
> +Disassembly of section .text:
> +
> +00000000 <.text>:
> +   0:	4af7                	sub_s	r15,r2,r15
> diff --git a/gas/testsuite/gas/arc/cpu-pseudop-2.s b/gas/testsuite/gas/arc/cpu-pseudop-2.s
> new file mode 100644
> index 0000000..def89d6
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/cpu-pseudop-2.s
> @@ -0,0 +1,5 @@
> +;;; Check if user can use additional command line options.
> +	.cpu EM
> +	.section	.text
> +
> +	sub_s r15,r2,r15	;code density instruction
> diff --git a/gas/testsuite/gas/arc/cpu-warn2.s b/gas/testsuite/gas/arc/cpu-warn2.s
> new file mode 100644
> index 0000000..e9ee338
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/cpu-warn2.s
> @@ -0,0 +1,4 @@
> +; Test warnings when multiple .cpu pseudo-ops are defined.
> +; { dg-do assemble }
> +	.cpu EM
> +	.cpu HS			;{ dg-error "Error: Multiple .cpu directives found" }
> -- 
> 1.9.1
>