This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Ping#2: [PATCH, resend] arm: fix extension feature disabling
- From: "Jan Beulich" <JBeulich at suse dot com>
- To: "Richard Earnshaw" <rearnsha at arm dot com>, "Nick Clifton" <nickc at redhat dot com>
- Cc: <marcus dot shawcroft at arm dot com>,<binutils at sourceware dot org>
- Date: Thu, 11 Dec 2014 10:13:52 +0000
- Subject: Ping#2: [PATCH, resend] arm: fix extension feature disabling
- Authentication-results: sourceware.org; auth=none
As was the case when having sent this first 1.5 years ago - no response
at all, also not after a first ping about two weeks ago. Not even a "this is
a bad idea, drop it" (in which case I'd expect at least an indication how to
reasonably achieve by other means what this patch arranges for). Marcus,
in the context of an ARM64 patch, actually suggested to resend this as he
apparently agrees that how things work currently isn't how they're meant
to work...
What is going on here?
Regards, Jan
>>> On 14.11.14 at 10:24, <JBeulich@suse.com> wrote:
> Using e.g.
>
> .arch_extension simd
> .arch_extension nocrypto
>
> so far results in SIMD support getting disabled, which I can't see being
> the purpose of the "no"-prefixed variants of architecture extension
> specifications.
>
> Of course it is questionable whether the current, counter intuitive
> behavior needs to be retained, and the new behavior perhaps be made work
> through e.g. a newly recognized "no-" prefix.
>
> gas/
> 2014-11-14 Jan Beulich <jbeulich@suse.com>
>
> * gas/config/tc-arm.c (struct arm_option_extension_value_table):
> Split field "value" into fields "merge_value" and "clear_value".
> (arm_extensions): Adjust initializer accordingly.
>
> --- 2014-11-14/gas/config/tc-arm.c
> +++ 2014-11-14/gas/config/tc-arm.c
> @@ -24493,40 +24493,51 @@ struct arm_option_extension_value_table
> {
> char *name;
> size_t name_len;
> - const arm_feature_set value;
> + const arm_feature_set merge_value;
> + const arm_feature_set clear_value;
> const arm_feature_set allowed_archs;
> };
>
> /* The following table must be in alphabetical order with a NULL last
> entry.
> */
> -#define ARM_EXT_OPT(N, V, AA) { N, sizeof (N) - 1, V, AA }
> +#define ARM_EXT_OPT(N, M, C, AA) { N, sizeof (N) - 1, M, C, AA }
> static const struct arm_option_extension_value_table arm_extensions[] =
> {
> - ARM_EXT_OPT ("crc", ARCH_CRC_ARMV8, ARM_FEATURE (ARM_EXT_V8, 0)),
> + ARM_EXT_OPT ("crc", ARCH_CRC_ARMV8, ARM_FEATURE (0, CRC_EXT_ARMV8),
> + ARM_FEATURE (ARM_EXT_V8, 0)),
> ARM_EXT_OPT ("crypto", FPU_ARCH_CRYPTO_NEON_VFP_ARMV8,
> + ARM_FEATURE (0, FPU_CRYPTO_ARMV8),
> ARM_FEATURE (ARM_EXT_V8, 0)),
> - ARM_EXT_OPT ("fp", FPU_ARCH_VFP_ARMV8,
> + ARM_EXT_OPT ("fp", FPU_ARCH_VFP_ARMV8, ARM_FEATURE (0,
> FPU_VFP_ARMV8),
> ARM_FEATURE (ARM_EXT_V8, 0)),
> ARM_EXT_OPT ("idiv", ARM_FEATURE (ARM_EXT_ADIV | ARM_EXT_DIV, 0),
> + ARM_FEATURE (ARM_EXT_ADIV | ARM_EXT_DIV, 0),
> ARM_FEATURE (ARM_EXT_V7A | ARM_EXT_V7R, 0)),
> - ARM_EXT_OPT ("iwmmxt",ARM_FEATURE (0, ARM_CEXT_IWMMXT), ARM_ANY),
> - ARM_EXT_OPT ("iwmmxt2",
> - ARM_FEATURE (0, ARM_CEXT_IWMMXT2), ARM_ANY),
> - ARM_EXT_OPT ("maverick",
> - ARM_FEATURE (0, ARM_CEXT_MAVERICK), ARM_ANY),
> + ARM_EXT_OPT ("iwmmxt",ARM_FEATURE (0, ARM_CEXT_IWMMXT),
> + ARM_FEATURE (0, ARM_CEXT_IWMMXT), ARM_ANY),
> + ARM_EXT_OPT ("iwmmxt2", ARM_FEATURE (0, ARM_CEXT_IWMMXT2),
> + ARM_FEATURE (0, ARM_CEXT_IWMMXT2), ARM_ANY),
> + ARM_EXT_OPT ("maverick", ARM_FEATURE (0, ARM_CEXT_MAVERICK),
> + ARM_FEATURE (0, ARM_CEXT_MAVERICK), ARM_ANY),
> ARM_EXT_OPT ("mp", ARM_FEATURE (ARM_EXT_MP, 0),
> + ARM_FEATURE (ARM_EXT_MP, 0),
> ARM_FEATURE (ARM_EXT_V7A | ARM_EXT_V7R, 0)),
> ARM_EXT_OPT ("simd", FPU_ARCH_NEON_VFP_ARMV8,
> + ARM_FEATURE(0, FPU_NEON_ARMV8),
> ARM_FEATURE (ARM_EXT_V8, 0)),
> ARM_EXT_OPT ("os", ARM_FEATURE (ARM_EXT_OS, 0),
> + ARM_FEATURE (ARM_EXT_OS, 0),
> ARM_FEATURE (ARM_EXT_V6M, 0)),
> ARM_EXT_OPT ("sec", ARM_FEATURE (ARM_EXT_SEC, 0),
> + ARM_FEATURE (ARM_EXT_SEC, 0),
> ARM_FEATURE (ARM_EXT_V6K | ARM_EXT_V7A, 0)),
> ARM_EXT_OPT ("virt", ARM_FEATURE (ARM_EXT_VIRT | ARM_EXT_ADIV
> | ARM_EXT_DIV, 0),
> + ARM_FEATURE (ARM_EXT_VIRT, 0),
> ARM_FEATURE (ARM_EXT_V7A, 0)),
> - ARM_EXT_OPT ("xscale",ARM_FEATURE (0, ARM_CEXT_XSCALE), ARM_ANY),
> - { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE }
> + ARM_EXT_OPT ("xscale",ARM_FEATURE (0, ARM_CEXT_XSCALE),
> + ARM_FEATURE (0, ARM_CEXT_XSCALE), ARM_ANY),
> + { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE, ARM_ARCH_NONE }
> };
> #undef ARM_EXT_OPT
>
> @@ -24701,9 +24712,9 @@ arm_parse_extension (char *str, const ar
>
> /* Add or remove the extension. */
> if (adding_value)
> - ARM_MERGE_FEATURE_SETS (*ext_set, *ext_set, opt->value);
> + ARM_MERGE_FEATURE_SETS (*ext_set, *ext_set, opt->merge_value);
> else
> - ARM_CLEAR_FEATURE (*ext_set, *ext_set, opt->value);
> + ARM_CLEAR_FEATURE (*ext_set, *ext_set, opt->clear_value);
>
> break;
> }
> @@ -25436,9 +25447,10 @@ s_arm_arch_extension (int ignored ATTRIB
> }
>
> if (adding_value)
> - ARM_MERGE_FEATURE_SETS (selected_cpu, selected_cpu, opt->value);
> + ARM_MERGE_FEATURE_SETS (selected_cpu, selected_cpu,
> + opt->merge_value);
> else
> - ARM_CLEAR_FEATURE (selected_cpu, selected_cpu, opt->value);
> + ARM_CLEAR_FEATURE (selected_cpu, selected_cpu, opt->clear_value);
>
> mcpu_cpu_opt = &selected_cpu;
> ARM_MERGE_FEATURE_SETS (cpu_variant, *mcpu_cpu_opt, *mfpu_opt);