public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH, ARM 6/7] Allow extension availability to depend on several architecture bits
@ 2015-12-17  2:55 Thomas Preud'homme
  2016-03-29 11:32 ` [PATCH, ARM 6/7, ping] " Thomas Preudhomme
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Preud'homme @ 2015-12-17  2:55 UTC (permalink / raw)
  To: binutils

Hi,

This patch is part of a patch series to add support for ARMv8-M[1] to binutils. This specific patch refactors extension parsing code to allow the addition of the DSP extension to ARMv8-M mainline in the next patch of the series. Due to ARMv8-M coming in two profiles (Baseline and Mainline), it is necessary to guard the availability of this extension upon the presence of the bits ARM_EXT_V8 and ARM_EXT2_V8M since depending only on the presence of ARM_AEXT2_V8M would allow the extension for ARMv8-M Baseline as well.

However, currently the extension parsing code interprets several bits being set in the allowed_arch as meaning that the extension should be allowed if any of the bits is set in the selected architecture. This patch changes the meaning of several bits being set and extends allowed_arch into an array to cater to the existing need of an extension being allowed for two different architectures.

[1] For a quick overview of ARMv8-M please refer to the initial cover letter.


ChangeLog entry is as follows:

*** gas/ChangeLog ***

2015-11-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/tc-arm.c 
        (struct arm_option_extension_value_table): Make allowed_archs an array
        with 2 entries.
        (ARM_EXT_OPT): Adapt to only fill the first entry of allowed_archs.
        (ARM_EXT_OPT2): New macro filling the two entries of allowed_archs.
        (arm_extensions): Use separate entries in allowed_archs when several
        archs are allowed to use an extension and change ARCH_ANY in
        ARM_ARCH_NONE in allowed_archs.
        (arm_parse_extension): Check that, for each allowed_archs entry, all
        bits are set in the current architecture, ignoring ARM_ANY entries.
        (s_arm_arch_extension): Likewise.


*** include/opcode/ChangeLog ***

2015-12-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * arm.h (ARM_CPU_HAS_FEATURE): Add comment.
        (ARM_FSET_CPU_SUBSET): Define macro.


Tests done:

* No regression under binutils testsuite
* Toolchain was built successfully with and without the ARMv8-M support patches[2] with the following multilib list: armv6-m,armv7-m,armv7e-m,cortex-m7,armv8-m.base,armv8-m.main. The code generation for crtbegin.o, crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a, libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a, libstdc++.a and libsupc++.a is unchanged for all targets supported before the patches.
* Thumb-1 (default arch and --with-mode=thumb) and Thumb-2 (--with-arch=armv7-a --with-mode=thumb) GCC bootstrap using binutils with this patch
* No GCC testsuite regression on fast model compared to ARMv6s-M (Baseline) or ARMv7-M (Mainline)

[2] including this one, the ld one and the GCC one

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 76357b93fde7df854d39468df1d74cfc0a22456c..007179cf34d68b7b096ade16805f806763abfcfa 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -24950,12 +24950,16 @@ struct arm_option_extension_value_table
   size_t name_len;
   const arm_feature_set merge_value;
   const arm_feature_set clear_value;
-  const arm_feature_set allowed_archs;
+  /* List of architectures for which an extension is available.  ARM_ARCH_NONE
+     indicates that an extension is available for all architectures while
+     ARM_ANY marks an empty entry.  */
+  const arm_feature_set allowed_archs[2];
 };
 
 /* The following table must be in alphabetical order with a NULL last entry.
    */
-#define ARM_EXT_OPT(N, M, C, AA) { N, sizeof (N) - 1, M, C, AA }
+#define ARM_EXT_OPT(N, M, C, AA) { N, sizeof (N) - 1, M, C, { AA, ARM_ANY } }
+#define ARM_EXT_OPT2(N, M, C, AA1, AA2) { N, sizeof (N) - 1, M, C, {AA1, AA2} }
 static const struct arm_option_extension_value_table arm_extensions[] =
 {
   ARM_EXT_OPT ("crc",  ARCH_CRC_ARMV8, ARM_FEATURE_COPROC (CRC_EXT_ARMV8),
@@ -24965,18 +24969,20 @@ static const struct arm_option_extension_value_table arm_extensions[] =
 				   ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
   ARM_EXT_OPT ("fp",     FPU_ARCH_VFP_ARMV8, ARM_FEATURE_COPROC (FPU_VFP_ARMV8),
 				   ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
-  ARM_EXT_OPT ("idiv",	ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
+  ARM_EXT_OPT2 ("idiv",	ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
-				   ARM_FEATURE_CORE_LOW (ARM_EXT_V7A | ARM_EXT_V7R)),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7R)),
   ARM_EXT_OPT ("iwmmxt",ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT),
-			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT), ARM_ANY),
+			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT), ARM_ARCH_NONE),
   ARM_EXT_OPT ("iwmmxt2", ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2),
-			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2), ARM_ANY),
+			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2), ARM_ARCH_NONE),
   ARM_EXT_OPT ("maverick", ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK),
-			ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK), ARM_ANY),
-  ARM_EXT_OPT ("mp",	ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
+			ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK), ARM_ARCH_NONE),
+  ARM_EXT_OPT2 ("mp",	ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
-				   ARM_FEATURE_CORE_LOW (ARM_EXT_V7A | ARM_EXT_V7R)),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7R)),
   ARM_EXT_OPT ("simd",   FPU_ARCH_NEON_VFP_ARMV8,
 			ARM_FEATURE_COPROC (FPU_NEON_ARMV8),
 				   ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
@@ -24986,9 +24992,10 @@ static const struct arm_option_extension_value_table arm_extensions[] =
   ARM_EXT_OPT ("pan",	ARM_FEATURE_CORE_HIGH (ARM_EXT2_PAN),
 			ARM_FEATURE (ARM_EXT_V8, ARM_EXT2_PAN, 0),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
-  ARM_EXT_OPT ("sec",	ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
+  ARM_EXT_OPT2 ("sec",	ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
-				   ARM_FEATURE_CORE_LOW (ARM_EXT_V6K | ARM_EXT_V7A)),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V6K),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A)),
   ARM_EXT_OPT ("virt",	ARM_FEATURE_CORE_LOW (ARM_EXT_VIRT | ARM_EXT_ADIV
 				     | ARM_EXT_DIV),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_VIRT),
@@ -24997,8 +25004,8 @@ static const struct arm_option_extension_value_table arm_extensions[] =
 			ARM_FEATURE_COPROC (FPU_NEON_ARMV8 | FPU_NEON_EXT_RDMA),
 				   ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
   ARM_EXT_OPT ("xscale",ARM_FEATURE_COPROC (ARM_CEXT_XSCALE),
-			ARM_FEATURE_COPROC (ARM_CEXT_XSCALE), ARM_ANY),
-  { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE, ARM_ARCH_NONE }
+			ARM_FEATURE_COPROC (ARM_CEXT_XSCALE), ARM_ARCH_NONE),
+  { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE, { ARM_ARCH_NONE, ARM_ARCH_NONE } }
 };
 #undef ARM_EXT_OPT
 
@@ -25105,6 +25112,7 @@ arm_parse_extension (char *str, const arm_feature_set **opt_p)
      or removing it (0) and only allowing it to change in the order
      -1 -> 1 -> 0.  */
   const struct arm_option_extension_value_table * opt = NULL;
+  const arm_feature_set arm_any = ARM_ANY;
   int adding_value = -1;
 
   /* Copy the feature set, so that we can modify it.  */
@@ -25169,8 +25177,18 @@ arm_parse_extension (char *str, const arm_feature_set **opt_p)
       for (; opt->name != NULL; opt++)
 	if (opt->name_len == len && strncmp (opt->name, str, len) == 0)
 	  {
+	    int i, nb_allowed_archs =
+	      sizeof (opt->allowed_archs) / sizeof (opt->allowed_archs[0]);
 	    /* Check we can apply the extension to this architecture.  */
-	    if (!ARM_CPU_HAS_FEATURE (*ext_set, opt->allowed_archs))
+	    for (i = 0; i < nb_allowed_archs; i++)
+	      {
+		/* Empty entry.  */
+		if (ARM_FEATURE_EQUAL (opt->allowed_archs[i], arm_any))
+		  continue;
+		if (ARM_FSET_CPU_SUBSET (opt->allowed_archs[i], *ext_set))
+		  break;
+	      }
+	    if (i == nb_allowed_archs)
 	      {
 		as_bad (_("extension does not apply to the base architecture"));
 		return FALSE;
@@ -25923,6 +25941,7 @@ static void
 s_arm_arch_extension (int ignored ATTRIBUTE_UNUSED)
 {
   const struct arm_option_extension_value_table *opt;
+  const arm_feature_set arm_any = ARM_ANY;
   char saved_char;
   char *name;
   int adding_value = 1;
@@ -25943,7 +25962,18 @@ s_arm_arch_extension (int ignored ATTRIBUTE_UNUSED)
   for (opt = arm_extensions; opt->name != NULL; opt++)
     if (streq (opt->name, name))
       {
-	if (!ARM_CPU_HAS_FEATURE (*mcpu_cpu_opt, opt->allowed_archs))
+	int i, nb_allowed_archs =
+	  sizeof (opt->allowed_archs) / sizeof (opt->allowed_archs[i]);
+	for (i = 0; i < nb_allowed_archs; i++)
+	  {
+	    /* Empty entry.  */
+	    if (ARM_FEATURE_EQUAL (opt->allowed_archs[i], arm_any))
+	      continue;
+	    if (ARM_FSET_CPU_SUBSET (opt->allowed_archs[i], *mcpu_cpu_opt))
+	      break;
+	  }
+
+	if (i == nb_allowed_archs)
 	  {
 	    as_bad (_("architectural extension `%s' is not allowed for the "
 		      "current base architecture"), name);
diff --git a/include/opcode/arm.h b/include/opcode/arm.h
index 0209f037ad8047993dc2a9bfc88536c3a66c59b2..28d72b46c157f45056883122794b0ef07e696454 100644
--- a/include/opcode/arm.h
+++ b/include/opcode/arm.h
@@ -319,11 +319,18 @@ typedef struct
   unsigned long coproc;
 } arm_feature_set;
 
+/* Returns whether one of the feature bits set in FEAT is also set in CPU.  */
 #define ARM_CPU_HAS_FEATURE(CPU,FEAT) \
   (((CPU).core[0] & (FEAT).core[0]) != 0 \
    || ((CPU).core[1] & (FEAT).core[1]) != 0 \
    || ((CPU).coproc & (FEAT).coproc) != 0)
 
+/* Tests whether the features of A are a subset of B.  */
+#define ARM_FSET_CPU_SUBSET(A,B) \
+  (((A).core[0] & (B).core[0]) == (A).core[0] \
+   && ((A).core[1] & (B).core[1]) == (A).core[1] \
+   && ((A).coproc & (B).coproc) == (A).coproc)
+
 #define ARM_CPU_IS_ANY(CPU) \
   ((CPU).core[0] == ((arm_feature_set)ARM_ANY).core[0] \
    && (CPU).core[1] == ((arm_feature_set)ARM_ANY).core[1])


Is this ok for the master branch?

Best regards,

Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, ARM 6/7, ping] Allow extension availability to depend on several architecture bits
  2015-12-17  2:55 [PATCH, ARM 6/7] Allow extension availability to depend on several architecture bits Thomas Preud'homme
@ 2016-03-29 11:32 ` Thomas Preudhomme
  2016-03-30 15:23   ` Nick Clifton
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Preudhomme @ 2016-03-29 11:32 UTC (permalink / raw)
  To: binutils

Ping?

On Thursday 17 December 2015 10:55:06 Thomas Preud'homme wrote:
> Hi,
> 
> This patch is part of a patch series to add support for ARMv8-M[1] to
> binutils. This specific patch refactors extension parsing code to allow the
> addition of the DSP extension to ARMv8-M mainline in the next patch of the
> series. Due to ARMv8-M coming in two profiles (Baseline and Mainline), it
> is necessary to guard the availability of this extension upon the presence
> of the bits ARM_EXT_V8 and ARM_EXT2_V8M since depending only on the
> presence of ARM_AEXT2_V8M would allow the extension for ARMv8-M Baseline as
> well.
> 
> However, currently the extension parsing code interprets several bits being
> set in the allowed_arch as meaning that the extension should be allowed if
> any of the bits is set in the selected architecture. This patch changes the
> meaning of several bits being set and extends allowed_arch into an array to
> cater to the existing need of an extension being allowed for two different
> architectures.
> 
> [1] For a quick overview of ARMv8-M please refer to the initial cover
> letter.
> 
> 
> ChangeLog entry is as follows:
> 
> *** gas/ChangeLog ***
> 
> 2015-11-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/tc-arm.c
>         (struct arm_option_extension_value_table): Make allowed_archs an
> array with 2 entries.
>         (ARM_EXT_OPT): Adapt to only fill the first entry of allowed_archs.
>         (ARM_EXT_OPT2): New macro filling the two entries of allowed_archs.
>         (arm_extensions): Use separate entries in allowed_archs when several
> archs are allowed to use an extension and change ARCH_ANY in ARM_ARCH_NONE
> in allowed_archs.
>         (arm_parse_extension): Check that, for each allowed_archs entry, all
> bits are set in the current architecture, ignoring ARM_ANY entries.
> (s_arm_arch_extension): Likewise.
> 
> 
> *** include/opcode/ChangeLog ***
> 
> 2015-12-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm.h (ARM_CPU_HAS_FEATURE): Add comment.
>         (ARM_FSET_CPU_SUBSET): Define macro.
> 
> 
> Tests done:
> 
> * No regression under binutils testsuite
> * Toolchain was built successfully with and without the ARMv8-M support
> patches[2] with the following multilib list:
> armv6-m,armv7-m,armv7e-m,cortex-m7,armv8-m.base,armv8-m.main. The code
> generation for crtbegin.o, crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a,
> libc.a, libg.a, libgloss-linux.a, libm.a, libnosys.a, librdimon.a,
> librdpmon.a, libstdc++.a and libsupc++.a is unchanged for all targets
> supported before the patches. * Thumb-1 (default arch and
> --with-mode=thumb) and Thumb-2 (--with-arch=armv7-a --with-mode=thumb) GCC
> bootstrap using binutils with this patch * No GCC testsuite regression on
> fast model compared to ARMv6s-M (Baseline) or ARMv7-M (Mainline)
> 
> [2] including this one, the ld one and the GCC one
> 
> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
> index
> 76357b93fde7df854d39468df1d74cfc0a22456c..007179cf34d68b7b096ade16805f80676
> 3abfcfa 100644 --- a/gas/config/tc-arm.c
> +++ b/gas/config/tc-arm.c
> @@ -24950,12 +24950,16 @@ struct arm_option_extension_value_table
>    size_t name_len;
>    const arm_feature_set merge_value;
>    const arm_feature_set clear_value;
> -  const arm_feature_set allowed_archs;
> +  /* List of architectures for which an extension is available. 
> ARM_ARCH_NONE +     indicates that an extension is available for all
> architectures while +     ARM_ANY marks an empty entry.  */
> +  const arm_feature_set allowed_archs[2];
>  };
> 
>  /* The following table must be in alphabetical order with a NULL last
> entry. */
> -#define ARM_EXT_OPT(N, M, C, AA) { N, sizeof (N) - 1, M, C, AA }
> +#define ARM_EXT_OPT(N, M, C, AA) { N, sizeof (N) - 1, M, C, { AA, ARM_ANY }
> } +#define ARM_EXT_OPT2(N, M, C, AA1, AA2) { N, sizeof (N) - 1, M, C, {AA1,
> AA2} } static const struct arm_option_extension_value_table
> arm_extensions[] = {
>    ARM_EXT_OPT ("crc",  ARCH_CRC_ARMV8, ARM_FEATURE_COPROC (CRC_EXT_ARMV8),
> @@ -24965,18 +24969,20 @@ static const struct
> arm_option_extension_value_table arm_extensions[] = ARM_FEATURE_CORE_LOW
> (ARM_EXT_V8)),
>    ARM_EXT_OPT ("fp",     FPU_ARCH_VFP_ARMV8, ARM_FEATURE_COPROC
> (FPU_VFP_ARMV8), ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
> -  ARM_EXT_OPT ("idiv",	ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
> +  ARM_EXT_OPT2 ("idiv",	ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | 
ARM_EXT_DIV),
>  			ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
> -				   ARM_FEATURE_CORE_LOW (ARM_EXT_V7A | ARM_EXT_V7R)),
> +			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A),
> +			ARM_FEATURE_CORE_LOW (ARM_EXT_V7R)),
>    ARM_EXT_OPT ("iwmmxt",ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT),
> -			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT), ARM_ANY),
> +			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT), ARM_ARCH_NONE),
>    ARM_EXT_OPT ("iwmmxt2", ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2),
> -			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2), ARM_ANY),
> +			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2), ARM_ARCH_NONE),
>    ARM_EXT_OPT ("maverick", ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK),
> -			ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK), ARM_ANY),
> -  ARM_EXT_OPT ("mp",	ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
> +			ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK), ARM_ARCH_NONE),
> +  ARM_EXT_OPT2 ("mp",	ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
>  			ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
> -				   ARM_FEATURE_CORE_LOW (ARM_EXT_V7A | ARM_EXT_V7R)),
> +			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A),
> +			ARM_FEATURE_CORE_LOW (ARM_EXT_V7R)),
>    ARM_EXT_OPT ("simd",   FPU_ARCH_NEON_VFP_ARMV8,
>  			ARM_FEATURE_COPROC (FPU_NEON_ARMV8),
>  				   ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
> @@ -24986,9 +24992,10 @@ static const struct
> arm_option_extension_value_table arm_extensions[] = ARM_EXT_OPT
> ("pan",	ARM_FEATURE_CORE_HIGH (ARM_EXT2_PAN),
>  			ARM_FEATURE (ARM_EXT_V8, ARM_EXT2_PAN, 0),
>  			ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
> -  ARM_EXT_OPT ("sec",	ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
> +  ARM_EXT_OPT2 ("sec",	ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
>  			ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
> -				   ARM_FEATURE_CORE_LOW (ARM_EXT_V6K | ARM_EXT_V7A)),
> +			ARM_FEATURE_CORE_LOW (ARM_EXT_V6K),
> +			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A)),
>    ARM_EXT_OPT ("virt",	ARM_FEATURE_CORE_LOW (ARM_EXT_VIRT | ARM_EXT_ADIV
> 
>  				     | ARM_EXT_DIV),
> 
>  			ARM_FEATURE_CORE_LOW (ARM_EXT_VIRT),
> @@ -24997,8 +25004,8 @@ static const struct arm_option_extension_value_table
> arm_extensions[] = ARM_FEATURE_COPROC (FPU_NEON_ARMV8 | FPU_NEON_EXT_RDMA),
>  				   ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
>    ARM_EXT_OPT ("xscale",ARM_FEATURE_COPROC (ARM_CEXT_XSCALE),
> -			ARM_FEATURE_COPROC (ARM_CEXT_XSCALE), ARM_ANY),
> -  { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE, ARM_ARCH_NONE }
> +			ARM_FEATURE_COPROC (ARM_CEXT_XSCALE), ARM_ARCH_NONE),
> +  { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE, { ARM_ARCH_NONE, ARM_ARCH_NONE }
> } };
>  #undef ARM_EXT_OPT
> 
> @@ -25105,6 +25112,7 @@ arm_parse_extension (char *str, const
> arm_feature_set **opt_p) or removing it (0) and only allowing it to change
> in the order -1 -> 1 -> 0.  */
>    const struct arm_option_extension_value_table * opt = NULL;
> +  const arm_feature_set arm_any = ARM_ANY;
>    int adding_value = -1;
> 
>    /* Copy the feature set, so that we can modify it.  */
> @@ -25169,8 +25177,18 @@ arm_parse_extension (char *str, const
> arm_feature_set **opt_p) for (; opt->name != NULL; opt++)
>  	if (opt->name_len == len && strncmp (opt->name, str, len) == 0)
>  	  {
> +	    int i, nb_allowed_archs =
> +	      sizeof (opt->allowed_archs) / sizeof (opt->allowed_archs[0]);
>  	    /* Check we can apply the extension to this architecture.  */
> -	    if (!ARM_CPU_HAS_FEATURE (*ext_set, opt->allowed_archs))
> +	    for (i = 0; i < nb_allowed_archs; i++)
> +	      {
> +		/* Empty entry.  */
> +		if (ARM_FEATURE_EQUAL (opt->allowed_archs[i], arm_any))
> +		  continue;
> +		if (ARM_FSET_CPU_SUBSET (opt->allowed_archs[i], *ext_set))
> +		  break;
> +	      }
> +	    if (i == nb_allowed_archs)
>  	      {
>  		as_bad (_("extension does not apply to the base architecture"));
>  		return FALSE;
> @@ -25923,6 +25941,7 @@ static void
>  s_arm_arch_extension (int ignored ATTRIBUTE_UNUSED)
>  {
>    const struct arm_option_extension_value_table *opt;
> +  const arm_feature_set arm_any = ARM_ANY;
>    char saved_char;
>    char *name;
>    int adding_value = 1;
> @@ -25943,7 +25962,18 @@ s_arm_arch_extension (int ignored ATTRIBUTE_UNUSED)
> for (opt = arm_extensions; opt->name != NULL; opt++)
>      if (streq (opt->name, name))
>        {
> -	if (!ARM_CPU_HAS_FEATURE (*mcpu_cpu_opt, opt->allowed_archs))
> +	int i, nb_allowed_archs =
> +	  sizeof (opt->allowed_archs) / sizeof (opt->allowed_archs[i]);
> +	for (i = 0; i < nb_allowed_archs; i++)
> +	  {
> +	    /* Empty entry.  */
> +	    if (ARM_FEATURE_EQUAL (opt->allowed_archs[i], arm_any))
> +	      continue;
> +	    if (ARM_FSET_CPU_SUBSET (opt->allowed_archs[i], *mcpu_cpu_opt))
> +	      break;
> +	  }
> +
> +	if (i == nb_allowed_archs)
>  	  {
>  	    as_bad (_("architectural extension `%s' is not allowed for the "
>  		      "current base architecture"), name);
> diff --git a/include/opcode/arm.h b/include/opcode/arm.h
> index
> 0209f037ad8047993dc2a9bfc88536c3a66c59b2..28d72b46c157f45056883122794b0ef07
> e696454 100644 --- a/include/opcode/arm.h
> +++ b/include/opcode/arm.h
> @@ -319,11 +319,18 @@ typedef struct
>    unsigned long coproc;
>  } arm_feature_set;
> 
> +/* Returns whether one of the feature bits set in FEAT is also set in CPU. 
> */ #define ARM_CPU_HAS_FEATURE(CPU,FEAT) \
>    (((CPU).core[0] & (FEAT).core[0]) != 0 \
> 
>     || ((CPU).core[1] & (FEAT).core[1]) != 0 \
>     || ((CPU).coproc & (FEAT).coproc) != 0)
> 
> +/* Tests whether the features of A are a subset of B.  */
> +#define ARM_FSET_CPU_SUBSET(A,B) \
> +  (((A).core[0] & (B).core[0]) == (A).core[0] \
> +   && ((A).core[1] & (B).core[1]) == (A).core[1] \
> +   && ((A).coproc & (B).coproc) == (A).coproc)
> +
>  #define ARM_CPU_IS_ANY(CPU) \
>    ((CPU).core[0] == ((arm_feature_set)ARM_ANY).core[0] \
>     && (CPU).core[1] == ((arm_feature_set)ARM_ANY).core[1])
> 
> 
> Is this ok for the master branch?
> 
> Best regards,
> 
> Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, ARM 6/7, ping] Allow extension availability to depend on several architecture bits
  2016-03-29 11:32 ` [PATCH, ARM 6/7, ping] " Thomas Preudhomme
@ 2016-03-30 15:23   ` Nick Clifton
  2016-05-10 15:27     ` Thomas Preudhomme
  0 siblings, 1 reply; 4+ messages in thread
From: Nick Clifton @ 2016-03-30 15:23 UTC (permalink / raw)
  To: Thomas Preudhomme, binutils

Hi Thomas,

> Ping?

oops - sorry.

>> *** gas/ChangeLog ***
>>
>> 2015-11-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * config/tc-arm.c
>>          (struct arm_option_extension_value_table): Make allowed_archs an
>> array with 2 entries.
>>          (ARM_EXT_OPT): Adapt to only fill the first entry of allowed_archs.
>>          (ARM_EXT_OPT2): New macro filling the two entries of allowed_archs.
>>          (arm_extensions): Use separate entries in allowed_archs when several
>> archs are allowed to use an extension and change ARCH_ANY in ARM_ARCH_NONE
>> in allowed_archs.
>>          (arm_parse_extension): Check that, for each allowed_archs entry, all
>> bits are set in the current architecture, ignoring ARM_ANY entries.
>> (s_arm_arch_extension): Likewise.
>>
>> *** include/opcode/ChangeLog ***
>>
>> 2015-12-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * arm.h (ARM_CPU_HAS_FEATURE): Add comment.
>>          (ARM_FSET_CPU_SUBSET): Define macro.

Approved - please apply.

Cheers
  Nick

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, ARM 6/7, ping] Allow extension availability to depend on several architecture bits
  2016-03-30 15:23   ` Nick Clifton
@ 2016-05-10 15:27     ` Thomas Preudhomme
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Preudhomme @ 2016-05-10 15:27 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

On Wednesday 30 March 2016 16:22:53 Nick Clifton wrote:
> Hi Thomas,
> 
> > Ping?
> 
> oops - sorry.
> 
> >> *** gas/ChangeLog ***
> >> 
> >> 2015-11-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >> 
> >>          * config/tc-arm.c
> >>          (struct arm_option_extension_value_table): Make allowed_archs an
> >> 
> >> array with 2 entries.
> >> 
> >>          (ARM_EXT_OPT): Adapt to only fill the first entry of
> >>          allowed_archs.
> >>          (ARM_EXT_OPT2): New macro filling the two entries of
> >>          allowed_archs.
> >>          (arm_extensions): Use separate entries in allowed_archs when
> >>          several
> >> 
> >> archs are allowed to use an extension and change ARCH_ANY in
> >> ARM_ARCH_NONE
> >> in allowed_archs.
> >> 
> >>          (arm_parse_extension): Check that, for each allowed_archs entry,
> >>          all
> >> 
> >> bits are set in the current architecture, ignoring ARM_ANY entries.
> >> (s_arm_arch_extension): Likewise.
> >> 
> >> *** include/opcode/ChangeLog ***
> >> 
> >> 2015-12-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >> 
> >>          * arm.h (ARM_CPU_HAS_FEATURE): Add comment.
> >>          (ARM_FSET_CPU_SUBSET): Define macro.
> 
> Approved - please apply.

Committed the following patch (rebased on recent master with latest changes to 
extensions). Updated ChangeLog entries are:

*** gas/ChangeLog ***

2015-11-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/tc-arm.c 
        (struct arm_option_extension_value_table): Make allowed_archs an array
        with 2 entries.
        (ARM_EXT_OPT): Adapt to only fill the first entry of allowed_archs.
        (ARM_EXT_OPT2): New macro filling the two entries of allowed_archs.
        (arm_extensions): Use separate entries in allowed_archs when several
        archs are allowed to use an extension and change ARCH_ANY in
        ARM_ARCH_NONE in allowed_archs.
        (arm_parse_extension): Check that, for each allowed_archs entry, all
        bits are set in the current architecture, ignoring ARM_ANY entries.
        (s_arm_arch_extension): Likewise.


*** include/opcode/ChangeLog ***

2016-04-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * arm.h (ARM_FSET_CPU_SUBSET): Define macro.


diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 
6a9d6558a2b5b4255e0d40ad466785549bdf7d7a..d4dee3f628a03f54d8911862e6fc07da22648e2b 
100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -25470,12 +25470,16 @@ struct arm_option_extension_value_table
   size_t name_len;
   const arm_feature_set merge_value;
   const arm_feature_set clear_value;
-  const arm_feature_set allowed_archs;
+  /* List of architectures for which an extension is available.  
ARM_ARCH_NONE
+     indicates that an extension is available for all architectures while
+     ARM_ANY marks an empty entry.  */
+  const arm_feature_set allowed_archs[2];
 };
 
 /* The following table must be in alphabetical order with a NULL last entry.
    */
-#define ARM_EXT_OPT(N, M, C, AA) { N, sizeof (N) - 1, M, C, AA }
+#define ARM_EXT_OPT(N, M, C, AA) { N, sizeof (N) - 1, M, C, { AA, ARM_ANY } }
+#define ARM_EXT_OPT2(N, M, C, AA1, AA2) { N, sizeof (N) - 1, M, C, {AA1, AA2} 
}
 static const struct arm_option_extension_value_table arm_extensions[] =
 {
   ARM_EXT_OPT ("crc",  ARCH_CRC_ARMV8, ARM_FEATURE_COPROC (CRC_EXT_ARMV8),
@@ -25488,18 +25492,20 @@ static const struct arm_option_extension_value_table 
arm_extensions[] =
   ARM_EXT_OPT ("fp16",  ARM_FEATURE_CORE_HIGH (ARM_EXT2_FP16_INST),
 			ARM_FEATURE_CORE_HIGH (ARM_EXT2_FP16_INST),
 			ARM_ARCH_V8_2A),
-  ARM_EXT_OPT ("idiv",	ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
+  ARM_EXT_OPT2 ("idiv",	ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_ADIV | ARM_EXT_DIV),
-				   ARM_FEATURE_CORE_LOW (ARM_EXT_V7A | ARM_EXT_V7R)),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7R)),
   ARM_EXT_OPT ("iwmmxt",ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT),
-			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT), ARM_ANY),
+			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT), ARM_ARCH_NONE),
   ARM_EXT_OPT ("iwmmxt2", ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2),
-			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2), ARM_ANY),
+			ARM_FEATURE_COPROC (ARM_CEXT_IWMMXT2), ARM_ARCH_NONE),
   ARM_EXT_OPT ("maverick", ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK),
-			ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK), ARM_ANY),
-  ARM_EXT_OPT ("mp",	ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
+			ARM_FEATURE_COPROC (ARM_CEXT_MAVERICK), ARM_ARCH_NONE),
+  ARM_EXT_OPT2 ("mp",	ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_MP),
-				   ARM_FEATURE_CORE_LOW (ARM_EXT_V7A | ARM_EXT_V7R)),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7R)),
   ARM_EXT_OPT ("os",	ARM_FEATURE_CORE_LOW (ARM_EXT_OS),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_OS),
 				   ARM_FEATURE_CORE_LOW (ARM_EXT_V6M)),
@@ -25509,9 +25515,10 @@ static const struct arm_option_extension_value_table 
arm_extensions[] =
   ARM_EXT_OPT ("rdma",  FPU_ARCH_NEON_VFP_ARMV8_1,
 			ARM_FEATURE_COPROC (FPU_NEON_ARMV8 | FPU_NEON_EXT_RDMA),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
-  ARM_EXT_OPT ("sec",	ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
+  ARM_EXT_OPT2 ("sec",	ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_SEC),
-				   ARM_FEATURE_CORE_LOW (ARM_EXT_V6K | ARM_EXT_V7A)),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V6K),
+			ARM_FEATURE_CORE_LOW (ARM_EXT_V7A)),
   ARM_EXT_OPT ("simd",  FPU_ARCH_NEON_VFP_ARMV8,
 			ARM_FEATURE_COPROC (FPU_NEON_ARMV8),
 			ARM_FEATURE_CORE_LOW (ARM_EXT_V8)),
@@ -25520,8 +25527,8 @@ static const struct arm_option_extension_value_table 
arm_extensions[] =
 			ARM_FEATURE_CORE_LOW (ARM_EXT_VIRT),
 				   ARM_FEATURE_CORE_LOW (ARM_EXT_V7A)),
   ARM_EXT_OPT ("xscale",ARM_FEATURE_COPROC (ARM_CEXT_XSCALE),
-			ARM_FEATURE_COPROC (ARM_CEXT_XSCALE), ARM_ANY),
-  { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE, ARM_ARCH_NONE }
+			ARM_FEATURE_COPROC (ARM_CEXT_XSCALE), ARM_ARCH_NONE),
+  { NULL, 0, ARM_ARCH_NONE, ARM_ARCH_NONE, { ARM_ARCH_NONE, ARM_ARCH_NONE } }
 };
 #undef ARM_EXT_OPT
 
@@ -25627,6 +25634,7 @@ arm_parse_extension (const char *str, const 
arm_feature_set **opt_p)
      or removing it (0) and only allowing it to change in the order
      -1 -> 1 -> 0.  */
   const struct arm_option_extension_value_table * opt = NULL;
+  const arm_feature_set arm_any = ARM_ANY;
   int adding_value = -1;
 
   /* Copy the feature set, so that we can modify it.  */
@@ -25691,8 +25699,18 @@ arm_parse_extension (const char *str, const 
arm_feature_set **opt_p)
       for (; opt->name != NULL; opt++)
 	if (opt->name_len == len && strncmp (opt->name, str, len) == 0)
 	  {
+	    int i, nb_allowed_archs =
+	      sizeof (opt->allowed_archs) / sizeof (opt->allowed_archs[0]);
 	    /* Check we can apply the extension to this architecture.  */
-	    if (!ARM_CPU_HAS_FEATURE (*ext_set, opt->allowed_archs))
+	    for (i = 0; i < nb_allowed_archs; i++)
+	      {
+		/* Empty entry.  */
+		if (ARM_FEATURE_EQUAL (opt->allowed_archs[i], arm_any))
+		  continue;
+		if (ARM_FSET_CPU_SUBSET (opt->allowed_archs[i], *ext_set))
+		  break;
+	      }
+	    if (i == nb_allowed_archs)
 	      {
 		as_bad (_("extension does not apply to the base architecture"));
 		return FALSE;
@@ -26448,6 +26466,7 @@ static void
 s_arm_arch_extension (int ignored ATTRIBUTE_UNUSED)
 {
   const struct arm_option_extension_value_table *opt;
+  const arm_feature_set arm_any = ARM_ANY;
   char saved_char;
   char *name;
   int adding_value = 1;
@@ -26468,7 +26487,18 @@ s_arm_arch_extension (int ignored ATTRIBUTE_UNUSED)
   for (opt = arm_extensions; opt->name != NULL; opt++)
     if (streq (opt->name, name))
       {
-	if (!ARM_CPU_HAS_FEATURE (*mcpu_cpu_opt, opt->allowed_archs))
+	int i, nb_allowed_archs =
+	  sizeof (opt->allowed_archs) / sizeof (opt->allowed_archs[i]);
+	for (i = 0; i < nb_allowed_archs; i++)
+	  {
+	    /* Empty entry.  */
+	    if (ARM_FEATURE_EQUAL (opt->allowed_archs[i], arm_any))
+	      continue;
+	    if (ARM_FSET_CPU_SUBSET (opt->allowed_archs[i], *mcpu_cpu_opt))
+	      break;
+	  }
+
+	if (i == nb_allowed_archs)
 	  {
 	    as_bad (_("architectural extension `%s' is not allowed for the "
 		      "current base architecture"), name);
diff --git a/include/opcode/arm.h b/include/opcode/arm.h
index 
36dcc20fdb2d5fc482529d566640cd0ae8ee4898..c8883dbac557e17b539f3a51c04aa07c65422e8d 
100644
--- a/include/opcode/arm.h
+++ b/include/opcode/arm.h
@@ -330,6 +330,12 @@ typedef struct
    || ((CPU).core[1] & (FEAT).core[1]) != 0 \
    || ((CPU).coproc & (FEAT).coproc) != 0)
 
+/* Tests whether the features of A are a subset of B.  */
+#define ARM_FSET_CPU_SUBSET(A,B) \
+  (((A).core[0] & (B).core[0]) == (A).core[0] \
+   && ((A).core[1] & (B).core[1]) == (A).core[1] \
+   && ((A).coproc & (B).coproc) == (A).coproc)
+
 #define ARM_CPU_IS_ANY(CPU) \
   ((CPU).core[0] == ((arm_feature_set)ARM_ANY).core[0] \
    && (CPU).core[1] == ((arm_feature_set)ARM_ANY).core[1])


Best regards,

Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-05-10 15:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-17  2:55 [PATCH, ARM 6/7] Allow extension availability to depend on several architecture bits Thomas Preud'homme
2016-03-29 11:32 ` [PATCH, ARM 6/7, ping] " Thomas Preudhomme
2016-03-30 15:23   ` Nick Clifton
2016-05-10 15:27     ` Thomas Preudhomme

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).