This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Fix and extend features of .cpu directive.
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>
- Cc: binutils at sourceware dot org, Francois dot Bedard at synopsys dot com, Anton dot Kolesov at synopsys dot com
- Date: Thu, 17 Nov 2016 19:23:37 +0000
- Subject: Re: [PATCH] Fix and extend features of .cpu directive.
- Authentication-results: sourceware.org; auth=none
- References: <1479391187-3537-1-git-send-email-claziss@synopsys.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
>