This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH, ARM 6/7] Allow extension availability to depend on several architecture bits


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]