The 02/21/2019 22:34, James Greenhalgh wrote:
> On Wed, Feb 20, 2019 at 08:00:38AM -0600, Tamar Christina wrote:
> > Hi All,
> > 
> > Commandline options on AArch64 don't get canonicalized into the smallest
> > possible set before output to the assembler. This means that overlapping 
> > feature
> > sets are emitted with superfluous parts.
> > 
> > Normally this isn't an issue, but in the case of crypto we have 
> > retro-actively
> > split it into aes and sha2. We need to emit only +crypto to the assembler
> > so old assemblers continue to work.
> > 
> > Because of how -mcpu=native and -march=native work they end up enabling all
> > feature bits. Instead we need to get the smallest possible set, which would 
> > also
> > fix the problem with older the assemblers and the retro-active split.
> > 
> > The function that handles this is called quite often.  It is called for any
> > push/pop options or attribute that changes arch, cpu etc.  In order to not 
> > make
> > this search for the smallest set too expensive we sort the options based on 
> > the
> > number of features (bits) they enable.  This allows us to process the list
> > linearly instead of quadratically (Once we have enabled a feature, we know 
> > that
> > anything else that enables it can be ignored.  By sorting we'll get the 
> > biggest
> > groups first and thus the smallest combination of commandline flags).
> > 
> > The Option handling structures have been extended to have a boolean to 
> > indicate
> > whether the option is synthetic, with that I mean if the option flag itself
> > enables a new feature.
> > 
> > e.g. +crypto isn't an actual feature, it just enables other features, but 
> > others
> > like +rdma enable multiple dependent features but is itself also a feature.
> > 
> > There are two ways to solve this.
> > 
> > 1) Either have the options that are feature bits also turn themselves on, 
> > e.g.
> >    change rdma to turn on FP, SIMD and RDMA as dependency bits.
> > 
> > 2) Make a distinction between these two different type of features and have 
> > the
> >    framework handle it correctly.
> > 
> > Even though it's more code I went for the second approach, as it's the one
> > that'll be less fragile (people can't forget it) and gives the least 
> > surprises.
> > 
> > Effectively this patch changes the following:
> > 
> > The values before the => are the old compiler and after the => the new code.
> > 
> > -march=armv8.2-a+crypto+sha2 => -march=armv8.2-a+crypto
> > -march=armv8.2-a+sha2+aes => -march=armv8.2-a+crypto
> > 
> > The remaining behaviors stay the same.
> > 
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> > 
> > Ok for trunk?
> 
> OK, but I don't understand why the CRC special case is needed. My copy of
> the Arm Architecture Reference Manual suggests that all versions of the
> architceture from ARmv8.1-A are required to implement the CRC32 extension.
> Is there some old assembler that doesn't honour that? Whatever is driving
> that requirement could usefully be added to the comments.

From my understanding it's because of a bug in an older assembler.  I am not
sure really what our support Window for this kind of stuff is to be honest.
But probably somewhere, someone will still be using the latest GCC with a
very old binutils.

I have updated the patch with a comment to this effect.  I have also made a
small change in splitting the loop that emits +<opt> and +no<opt> into two
different ones.  No change aside from the copied "for..".  The reason for this
is that the assemblers expect all + before +no, as a final full toolchain
regtest pointed out.

I have committed this as trivial as no change was done to the working of the
options loop. And also added a comment to this requirement in the code.

Thanks,
Tamar

> 
> I find it very hard to believe that this code is what we need for correct
> behaviour on AArch64; this level of complexity implies we're doing something
> very wrong with either the definition or the implementation of these features
> bits which makes it hard for us to maintain, and hard for users and dependent
> tools (e.g. assemblers) to know what to expect from the compiler.
> 
> I've seen a number of bugs in this code recently. While I appreciate your
> patch for fixing one of them, I find the cases and expectations so hard
> to reason about that I can't say I am sure we are now bug free.
> 
> OK for trunk as an improvement over today, and to help us get towards a
> release; but I'm very unhappy with this corner of the compiler!
> 
> Thanks,
> James
> 
> > gcc/ChangeLog:
> > 
> > 2019-02-20  Tamar Christina  <tamar.christ...@arm.com>
> > 
> >     PR target/88530
> >     * common/config/aarch64/aarch64-common.c
> >     (struct aarch64_option_extension): Add is_synthetic.
> >     (all_extensions): Use it.
> >     (TARGET_OPTION_INIT_STRUCT): Define hook.
> >     (struct gcc_targetm_common): Moved to end.
> >     (all_extensions_by_on): New.
> >     (opt_ext_cmp, typedef opt_ext): New.
> >     (aarch64_option_init_struct): New.
> >     (aarch64_contains_opt): New.
> >     (aarch64_get_extension_string_for_isa_flags): Output smallest set.
> >     * config/aarch64/aarch64-option-extensions.def
> >     (AARCH64_OPT_EXTENSION): Explicitly include AES and SHA2 in crypto.
> >     (fp, simd, crc, lse, fp16, rcpc, rdma, dotprod, aes, sha2, sha3,
> >     sm4, fp16fml, sve, profile, rng, memtag, sb, ssbs, predres):
> >     Set is_synthetic to false.
> >     (crypto): Set is_synthetic to true.
> >     * config/aarch64/driver-aarch64.c (AARCH64_OPT_EXTENSION): Add
> >     SYNTHETIC.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 2019-02-20  Tamar Christina  <tamar.christ...@arm.com>
> > 
> >     PR target/88530
> >     * gcc.target/aarch64/options_set_1.c: New test.
> >     * gcc.target/aarch64/options_set_2.c: New test.
> >     * gcc.target/aarch64/options_set_3.c: New test.
> >     * gcc.target/aarch64/options_set_4.c: New test.
> >     * gcc.target/aarch64/options_set_5.c: New test.
> >     * gcc.target/aarch64/options_set_6.c: New test.
> >     * gcc.target/aarch64/options_set_7.c: New test.
> >     * gcc.target/aarch64/options_set_8.c: New test.
> >     * gcc.target/aarch64/options_set_9.c: New test.
> > 
> > -- 
> 

-- 
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index fd870e187a6634b929bc058f99e768e829008179..5329471dc574b745c50c5a9b1000e749157b87f4 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -46,6 +46,8 @@
 #define TARGET_OPTION_DEFAULT_PARAMS aarch64_option_default_params
 #undef TARGET_OPTION_VALIDATE_PARAM
 #define TARGET_OPTION_VALIDATE_PARAM aarch64_option_validate_param
+#undef TARGET_OPTION_INIT_STRUCT
+#define TARGET_OPTION_INIT_STRUCT aarch64_option_init_struct
 
 /* Set default optimization options.  */
 static const struct default_options aarch_option_optimization_table[] =
@@ -164,8 +166,6 @@ aarch64_handle_option (struct gcc_options *opts,
     }
 }
 
-struct gcc_targetm_common targetm_common = TARGETM_COMMON_INITIALIZER;
-
 /* An ISA extension in the co-processor and main instruction set space.  */
 struct aarch64_option_extension
 {
@@ -173,15 +173,28 @@ struct aarch64_option_extension
   const unsigned long flag_canonical;
   const unsigned long flags_on;
   const unsigned long flags_off;
+  const bool is_synthetic;
 };
 
 /* ISA extensions in AArch64.  */
 static const struct aarch64_option_extension all_extensions[] =
 {
-#define AARCH64_OPT_EXTENSION(NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, Z) \
-  {NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF},
+#define AARCH64_OPT_EXTENSION(NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, \
+			      SYNTHETIC, Z) \
+  {NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, SYNTHETIC},
+#include "config/aarch64/aarch64-option-extensions.def"
+  {NULL, 0, 0, 0, false}
+};
+
+/* A copy of the ISA extensions list for AArch64 sorted by the popcount of
+   bits and extension turned on.  Cached for efficiency.  */
+static struct aarch64_option_extension all_extensions_by_on[] =
+{
+#define AARCH64_OPT_EXTENSION(NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, \
+			      SYNTHETIC, Z) \
+  {NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, SYNTHETIC},
 #include "config/aarch64/aarch64-option-extensions.def"
-  {NULL, 0, 0, 0}
+  {NULL, 0, 0, 0, false}
 };
 
 struct processor_name_to_arch
@@ -298,6 +311,76 @@ aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
     candidates->safe_push (opt->name);
 }
 
+/* Comparer to sort aarch64's feature extensions by population count. Largest
+   first.  */
+
+typedef const struct aarch64_option_extension opt_ext;
+
+int opt_ext_cmp (const void* a, const void* b)
+{
+  opt_ext *opt_a = (opt_ext *)a;
+  opt_ext *opt_b = (opt_ext *)b;
+
+  /* We consider the total set of bits an options turns on to be the union of
+     the singleton set containing the option itself and the set of options it
+     turns on as a dependency.  As an example +dotprod turns on FL_DOTPROD and
+     FL_SIMD.  As such the set of bits represented by this option is
+     {FL_DOTPROD, FL_SIMD}. */
+  unsigned long total_flags_a = opt_a->flag_canonical & opt_a->flags_on;
+  unsigned long total_flags_b = opt_b->flag_canonical & opt_b->flags_on;
+  int popcnt_a = popcount_hwi ((HOST_WIDE_INT)total_flags_a);
+  int popcnt_b = popcount_hwi ((HOST_WIDE_INT)total_flags_b);
+  int order = popcnt_b - popcnt_a;
+
+  /* If they have the same amount of bits set, give it a more
+     deterministic ordering by using the value of the bits themselves.  */
+  if (order == 0)
+    return total_flags_b - total_flags_a;
+
+  return order;
+}
+
+/* Implement TARGET_OPTION_INIT_STRUCT.  */
+
+static void
+aarch64_option_init_struct (struct gcc_options *opts ATTRIBUTE_UNUSED)
+{
+    /* Sort the extensions based on how many bits they set, order the larger
+       counts first.  We sort the list because this makes processing the
+       feature bits O(n) instead of O(n^2).  While n is small, the function
+       to calculate the feature strings is called on every options push,
+       pop and attribute change (arm_neon headers, lto etc all cause this to
+       happen quite frequently).  It is a trade-off between time and space and
+       so time won.  */
+    int n_extensions
+      = sizeof (all_extensions) / sizeof (struct aarch64_option_extension);
+    qsort (&all_extensions_by_on, n_extensions,
+	   sizeof (struct aarch64_option_extension), opt_ext_cmp);
+}
+
+/* Checks to see if enough bits from the option OPT are enabled in
+   ISA_FLAG_BITS to be able to replace the individual options with the
+   canonicalized version of the option.  This is done based on two rules:
+
+   1) Synthetic groups, such as +crypto we only care about the bits that are
+      turned on. e.g. +aes+sha2 can be replaced with +crypto.
+
+   2) Options that themselves have a bit, such as +rdma, in this case, all the
+      feature bits they turn on must be available and the bit for the option
+      itself must be.  In this case it's effectively a reduction rather than a
+      grouping. e.g. +fp+simd is not enough to turn on +rdma, for that you would
+      need +rdma+fp+simd which is reduced down to +rdma.
+*/
+
+static bool
+aarch64_contains_opt (unsigned long isa_flag_bits, opt_ext *opt)
+{
+  unsigned long flags_check
+    = opt->is_synthetic ? opt->flags_on : opt->flag_canonical;
+
+  return (isa_flag_bits & flags_check) == flags_check;
+}
+
 /* Return a string representation of ISA_FLAGS.  DEFAULT_ARCH_FLAGS
    gives the default set of flags which are implied by whatever -march
    we'd put out.  Our job is to figure out the minimal set of "+" and
@@ -311,26 +394,97 @@ aarch64_get_extension_string_for_isa_flags (unsigned long isa_flags,
   const struct aarch64_option_extension *opt = NULL;
   std::string outstr = "";
 
-  /* Pass one: Find all the things we need to turn on.  As a special case,
-     we always want to put out +crc if it is enabled.  */
-  for (opt = all_extensions; opt->name != NULL; opt++)
-    if ((isa_flags & opt->flag_canonical
-	 && !(default_arch_flags & opt->flag_canonical))
-	|| (default_arch_flags & opt->flag_canonical
-            && opt->flag_canonical == AARCH64_ISA_CRC))
-      {
-	outstr += "+";
-	outstr += opt->name;
-      }
+  unsigned long isa_flag_bits = isa_flags;
 
-  /* Pass two: Find all the things we need to turn off.  */
-  for (opt = all_extensions; opt->name != NULL; opt++)
-    if ((~isa_flags) & opt->flag_canonical
-	&& !((~default_arch_flags) & opt->flag_canonical))
+  /* Pass one: Minimize the search space by reducing the set of options
+     to the smallest set that still turns on the same features as before in
+     conjunction with the bits that are turned on by default for the selected
+     architecture.  */
+  for (opt = all_extensions_by_on; opt->name != NULL; opt++)
+    {
+      /* If the bit is on by default, then all the options it turns on are also
+	 on by default due to the transitive dependencies.
+
+         If the option is enabled explicitly in the set then we need to emit
+	 an option for it.  Since this list is sorted by extensions setting the
+	 largest number of featers first, we can be sure that nothing else will
+	 ever need to set the bits we already set.  Consider the following
+	 situation:
+
+	  Feat1 = A + B + C
+	  Feat2 = A + B
+	  Feat3 = A + D
+	  Feat4 = B + C
+	  Feat5 = C
+
+	The following results are expected:
+
+	  A + C = A + Feat5
+	  B + C = Feat4
+	  Feat4 + A = Feat1
+	  Feat2 + Feat5 = Feat1
+	  Feat1 + C = Feat1
+          Feat3 + Feat4 = Feat1 + D
+
+	This search assumes that all invidual feature bits are use visible,
+	in other words the user must be able to do +A, +B, +C and +D.  */
+      if (aarch64_contains_opt (isa_flag_bits | default_arch_flags, opt))
       {
-	outstr += "+no";
-	outstr += opt->name;
+	/* We remove all the dependent bits, to prevent them from being turned
+	   on twice.  This only works because we assume that all there are
+	   individual options to set all bits standalone.  */
+	isa_flag_bits &= ~opt->flags_on;
+	isa_flag_bits |= opt->flag_canonical;
       }
+    }
+
+   /* By toggling bits on and off, we may have set bits on that are already
+      enabled by default.  So we mask the default set out so we don't emit an
+      option for them.  Instead of checking for this each time during Pass One
+      we just mask all default bits away at the end.  */
+   isa_flag_bits &= ~default_arch_flags;
+
+   /* We now have the smallest set of features we need to process.  A subsequent
+      linear scan of the bits in isa_flag_bits will allow us to print the ext
+      names.  However as a special case if CRC was enabled before, always print
+      it.  This is required because some CPUs have an incorrect specification
+      in older assemblers.  Even though CRC should be the default for these
+      cases the -mcpu values won't turn it on.  */
+  if (isa_flags & AARCH64_ISA_CRC)
+    isa_flag_bits |= AARCH64_ISA_CRC;
+
+  /* Pass Two:
+     Print the option names that we're sure we must turn on.  These are only
+     optional extension names.  Mandatory ones have already been removed and
+     ones we explicitly want off have been too.  */
+  for (opt = all_extensions_by_on; opt->name != NULL; opt++)
+    {
+      if (isa_flag_bits & opt->flag_canonical)
+	{
+	  outstr += "+";
+	  outstr += opt->name;
+	}
+    }
+
+  /* Pass Three:
+     Print out a +no for any mandatory extension that we are
+     turning off.  By this point aarch64_parse_extension would have ensured
+     that any optional extensions are turned off.  The only things left are
+     things that can't be turned off usually, e.g. something that is on by
+     default because it's mandatory and we want it off.  For turning off bits
+     we don't guarantee the smallest set of flags, but instead just emit all
+     options the user has specified.
+
+     The assembler requires all +<opts> to be printed before +no<opts>.  */
+  for (opt = all_extensions_by_on; opt->name != NULL; opt++)
+    {
+      if ((~isa_flags) & opt->flag_canonical
+		&& !((~default_arch_flags) & opt->flag_canonical))
+	{
+	  outstr += "+no";
+	  outstr += opt->name;
+	}
+    }
 
   return outstr;
 }
@@ -411,5 +565,7 @@ aarch64_rewrite_mcpu (int argc, const char **argv)
   return aarch64_rewrite_selected_cpu (argv[argc - 1]);
 }
 
+struct gcc_targetm_common targetm_common = TARGETM_COMMON_INITIALIZER;
+
 #undef AARCH64_CPU_NAME_LENGTH
 
diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
index 2879e35cf2d41d96cb41bb3edd82c0f50091b077..1b2f4abbd5b850135af2cb7010921b45c03516a9 100644
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -21,29 +21,38 @@
 
    Before using #include to read this file, define a macro:
 
-      AARCH64_OPT_EXTENSION(EXT_NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, FEATURE_STRING)
-
-   EXT_NAME is the name of the extension, represented as a string constant.
-   FLAGS_CANONICAL is the canonical internal name for this flag.
-   FLAGS_ON are the bitwise-or of the features that enabling the extension
-   adds, or zero if enabling this extension has no effect on other features.
-   FLAGS_OFF are the bitwise-or of the features that disabling the extension
-   removes, or zero if disabling this extension has no effect on other
-   features.
-   FEAT_STRING is a string containing the entries in the 'Features' field of
-   /proc/cpuinfo on a GNU/Linux system that correspond to this architecture
-   extension being available.  Sometimes multiple entries are needed to enable
-   the extension (for example, the 'crypto' extension depends on four
-   entries: aes, pmull, sha1, sha2 being present).  In that case this field
-   should contain a space (" ") separated list of the strings in 'Features'
-   that are required.  Their order is not important.  */
+      AARCH64_OPT_EXTENSION(EXT_NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF,
+			    SYNTHETIC, FEATURE_STRING)
+
+   - EXT_NAME is the name of the extension, represented as a string constant.
+   - FLAGS_CANONICAL is the canonical internal name for this flag.
+   - FLAGS_ON are the bitwise-or of the features that enabling the extension
+     adds, or zero if enabling this extension has no effect on other features.
+   - FLAGS_OFF are the bitwise-or of the features that disabling the extension
+     removes, or zero if disabling this extension has no effect on other
+     features.
+   - SYNTHETIC is a boolean to indicate whether the option is a purely synthetic
+     grouping of options and that the option itself has no feature bit (e.g.
+     crypto).  This is used to determine when sum of the individual options in
+     FLAGS_ON can be replaced by FLAG_CANONICAL in options minimization.  If the
+     group is synthetic then they can be replaced when all options in FLAGS_ON
+     are enabled, otherwise they can only be replaced when
+     FLAGS_ON | FLAG_CANONICAL are enabled.
+   - FEAT_STRING is a string containing the entries in the 'Features' field of
+     /proc/cpuinfo on a GNU/Linux system that correspond to this architecture
+     extension being available.  Sometimes multiple entries are needed to enable
+     the extension (for example, the 'crypto' extension depends on four
+     entries: aes, pmull, sha1, sha2 being present).  In that case this field
+     should contain a space (" ") separated list of the strings in 'Features'
+     that are required.  Their order is not important.  */
 
 /* Enabling "fp" just enables "fp".
    Disabling "fp" also disables "simd", "crypto", "fp16", "aes", "sha2",
    "sha3", sm3/sm4 and "sve".  */
 AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | AARCH64_FL_CRYPTO |\
 		      AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
-		      AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, "fp")
+		      AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, \
+		      false, "fp")
 
 /* Enabling "simd" also enables "fp".
    Disabling "simd" also disables "crypto", "dotprod", "aes", "sha2", "sha3",
@@ -51,76 +60,86 @@ AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | AARCH64_FL_CRYPT
 AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, AARCH64_FL_CRYPTO |\
 		      AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 |\
 		      AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE,
-		      "asimd")
+		      false, "asimd")
 
-/* Enabling "crypto" also enables "fp" and "simd".
+/* Enabling "crypto" also enables "fp", "simd", "aes" and "sha2".
    Disabling "crypto" disables "crypto", "aes", "sha2", "sha3" and "sm3/sm4".  */
-AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | AARCH64_FL_SIMD,\
+AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO,
+		      AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_AES |\
+		      AARCH64_FL_SHA2,\
 		      AARCH64_FL_AES | AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | AARCH64_FL_SM4,\
-		      "aes pmull sha1 sha2")
+		      true, "aes pmull sha1 sha2")
 
 /* Enabling or disabling "crc" only changes "crc".  */
-AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, "crc32")
+AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, false, "crc32")
 
 /* Enabling or disabling "lse" only changes "lse".  */
-AARCH64_OPT_EXTENSION("lse", AARCH64_FL_LSE, 0, 0, "atomics")
+AARCH64_OPT_EXTENSION("lse", AARCH64_FL_LSE, 0, 0, false, "atomics")
 
 /* Enabling "fp16" also enables "fp".
    Disabling "fp16" disables "fp16", "fp16fml" and "sve".  */
 AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP,
-		      AARCH64_FL_F16FML | AARCH64_FL_SVE, "fphp asimdhp")
+		      AARCH64_FL_F16FML | AARCH64_FL_SVE, false, "fphp asimdhp")
 
 /* Enabling or disabling "rcpc" only changes "rcpc".  */
-AARCH64_OPT_EXTENSION("rcpc", AARCH64_FL_RCPC, 0, 0, "lrcpc")
+AARCH64_OPT_EXTENSION("rcpc", AARCH64_FL_RCPC, 0, 0, false, "lrcpc")
 
 /* Enabling "rdma" also enables "fp", "simd".
    Disabling "rdma" just disables "rdma".  */
-AARCH64_OPT_EXTENSION("rdma", AARCH64_FL_RDMA, AARCH64_FL_FP | AARCH64_FL_SIMD, 0, "asimdrdm")
+AARCH64_OPT_EXTENSION("rdma", AARCH64_FL_RDMA, \
+		      AARCH64_FL_FP | AARCH64_FL_SIMD, 0, false, "asimdrdm")
 
 /* Enabling "dotprod" also enables "simd".
    Disabling "dotprod" only disables "dotprod".  */
-AARCH64_OPT_EXTENSION("dotprod", AARCH64_FL_DOTPROD, AARCH64_FL_SIMD, 0, "asimddp")
+AARCH64_OPT_EXTENSION("dotprod", AARCH64_FL_DOTPROD, AARCH64_FL_SIMD, 0, \
+		      false, "asimddp")
 
 /* Enabling "aes" also enables "simd".
    Disabling "aes" just disables "aes".  */
-AARCH64_OPT_EXTENSION("aes", AARCH64_FL_AES, AARCH64_FL_SIMD, 0, "aes")
+AARCH64_OPT_EXTENSION("aes", AARCH64_FL_AES, AARCH64_FL_SIMD, 0, false, "aes")
 
 /* Enabling "sha2" also enables "simd".
    Disabling "sha2" just disables "sha2".  */
-AARCH64_OPT_EXTENSION("sha2", AARCH64_FL_SHA2, AARCH64_FL_SIMD, 0, "sha1 sha2")
+AARCH64_OPT_EXTENSION("sha2", AARCH64_FL_SHA2, AARCH64_FL_SIMD, 0, false, \
+		      "sha1 sha2")
 
 /* Enabling "sha3" enables "simd" and "sha2".
    Disabling "sha3" just disables "sha3".  */
-AARCH64_OPT_EXTENSION("sha3", AARCH64_FL_SHA3, AARCH64_FL_SIMD | AARCH64_FL_SHA2, 0, "sha3 sha512")
+AARCH64_OPT_EXTENSION("sha3", AARCH64_FL_SHA3, \
+		      AARCH64_FL_SIMD | AARCH64_FL_SHA2, 0, false, \
+		      "sha3 sha512")
 
 /* Enabling "sm4" also enables "simd".
    Disabling "sm4" just disables "sm4".  */
-AARCH64_OPT_EXTENSION("sm4", AARCH64_FL_SM4, AARCH64_FL_SIMD, 0, "sm3 sm4")
+AARCH64_OPT_EXTENSION("sm4", AARCH64_FL_SM4, AARCH64_FL_SIMD, 0, false, \
+		      "sm3 sm4")
 
 /* Enabling "fp16fml" also enables "fp" and "fp16".
    Disabling "fp16fml" just disables "fp16fml".  */
-AARCH64_OPT_EXTENSION("fp16fml", AARCH64_FL_F16FML, AARCH64_FL_FP | AARCH64_FL_F16, 0, "asimdfml")
+AARCH64_OPT_EXTENSION("fp16fml", AARCH64_FL_F16FML, \
+		      AARCH64_FL_FP | AARCH64_FL_F16, 0, false, "asimdfml")
 
 /* Enabling "sve" also enables "fp16", "fp" and "simd".
    Disabling "sve" just disables "sve".  */
-AARCH64_OPT_EXTENSION("sve", AARCH64_FL_SVE, AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_F16, 0, "sve")
+AARCH64_OPT_EXTENSION("sve", AARCH64_FL_SVE, AARCH64_FL_FP | AARCH64_FL_SIMD | \
+		      AARCH64_FL_F16, 0, false, "sve")
 
 /* Enabling/Disabling "profile" does not enable/disable any other feature.  */
-AARCH64_OPT_EXTENSION("profile", AARCH64_FL_PROFILE, 0, 0, "")
+AARCH64_OPT_EXTENSION("profile", AARCH64_FL_PROFILE, 0, 0, false, "")
 
 /* Enabling/Disabling "rng" only changes "rng".  */
-AARCH64_OPT_EXTENSION("rng", AARCH64_FL_RNG, 0, 0, "")
+AARCH64_OPT_EXTENSION("rng", AARCH64_FL_RNG, 0, 0, false, "")
 
 /* Enabling/Disabling "memtag" only changes "memtag".  */
-AARCH64_OPT_EXTENSION("memtag", AARCH64_FL_MEMTAG, 0, 0, "")
+AARCH64_OPT_EXTENSION("memtag", AARCH64_FL_MEMTAG, 0, 0, false, "")
 
 /* Enabling/Disabling "sb" only changes "sb".  */
-AARCH64_OPT_EXTENSION("sb", AARCH64_FL_SB, 0, 0, "")
+AARCH64_OPT_EXTENSION("sb", AARCH64_FL_SB, 0, 0, false, "")
 
 /* Enabling/Disabling "ssbs" only changes "ssbs".  */
-AARCH64_OPT_EXTENSION("ssbs", AARCH64_FL_SSBS, 0, 0, "")
+AARCH64_OPT_EXTENSION("ssbs", AARCH64_FL_SSBS, 0, 0, false, "")
 
 /* Enabling/Disabling "predres" only changes "predres".  */
-AARCH64_OPT_EXTENSION("predres", AARCH64_FL_PREDRES, 0, 0, "")
+AARCH64_OPT_EXTENSION("predres", AARCH64_FL_PREDRES, 0, 0, false, "")
 
 #undef AARCH64_OPT_EXTENSION
diff --git a/gcc/config/aarch64/driver-aarch64.c b/gcc/config/aarch64/driver-aarch64.c
index 100e0c3529cee21a7c07122d9eea33c8dca83aca..6051443d9268c0d1d4c024be8ba0731c03340e59 100644
--- a/gcc/config/aarch64/driver-aarch64.c
+++ b/gcc/config/aarch64/driver-aarch64.c
@@ -36,7 +36,8 @@ struct aarch64_arch_extension
   const char *feat_string;
 };
 
-#define AARCH64_OPT_EXTENSION(EXT_NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, FEATURE_STRING) \
+#define AARCH64_OPT_EXTENSION(EXT_NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, \
+			      SYNTHETIC, FEATURE_STRING) \
   { EXT_NAME, FLAG_CANONICAL, FEATURE_STRING },
 static struct aarch64_arch_extension aarch64_extensions[] =
 {
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_1.c b/gcc/testsuite/gcc.target/aarch64/options_set_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..40d9a05c905eb07103d3b437b5c1351e8620ab33
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_1.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.2-a" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crc} 1 } } */
+
+/* Check to see if crc is output by default.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_2.c b/gcc/testsuite/gcc.target/aarch64/options_set_2.c
new file mode 100644
index 0000000000000000000000000000000000000000..3476febce706b34430682e879a4aa3aac8f752db
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_2.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.2-a+crypto" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crypto\+crc} 1 } } */
+
+/* Check to see if crc and crypto are maintained if crypto specified.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_3.c b/gcc/testsuite/gcc.target/aarch64/options_set_3.c
new file mode 100644
index 0000000000000000000000000000000000000000..4558339f16c19555801899c357c50cedb23c28b0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_3.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.2-a+aes+sha2+crypto" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crypto\+crc} 1 } } */
+
+/* Check if smallest set is maintained when outputting. */
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_4.c b/gcc/testsuite/gcc.target/aarch64/options_set_4.c
new file mode 100644
index 0000000000000000000000000000000000000000..15514bfe93e61e63cbce1262ee951358cd22d6ce
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_4.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.2-a+aes+sha2" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crypto\+crc} 1 } } */
+
+/* Check if individual bits that make up a grouping is specified that only the
+   grouping is kept. */
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_5.c b/gcc/testsuite/gcc.target/aarch64/options_set_5.c
new file mode 100644
index 0000000000000000000000000000000000000000..b4c0901195ede4fe0dd71fbe02a47c35e9dedbbd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_5.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.2-a+aes+sha2+nosha2" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crc\+aes} 1 } } */
+
+/* Check if turning off feature bits works correctly and grouping is no
+   longer valid.   */
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_6.c b/gcc/testsuite/gcc.target/aarch64/options_set_6.c
new file mode 100644
index 0000000000000000000000000000000000000000..90a055928a273f06e08124a250e3107ad0704e47
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_6.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.2-a+crypto+nosha2" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crypto\+crc} 1 } } */
+
+/* Group as a whole was requested to be turned on, crypto itself is a bit and so
+   just turning off one feature can't turn it off.   */
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_7.c b/gcc/testsuite/gcc.target/aarch64/options_set_7.c
new file mode 100644
index 0000000000000000000000000000000000000000..71a2c8a19058c0ec25546085076503d206129e10
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_7.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.4-a+dotprod" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.4\-a} 1 } } */
+
+/* Checking if enabling default features drops the superfluous bits.   */
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_8.c b/gcc/testsuite/gcc.target/aarch64/options_set_8.c
new file mode 100644
index 0000000000000000000000000000000000000000..83be1bd7a5c3f2bc8d11a14f2c16415c6a7056f2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_8.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.4-a+nodotprod" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\.4\-a} 1 } } */
+
+/* Checking if trying to turn off default features propagates the commandline
+   option.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_9.c b/gcc/testsuite/gcc.target/aarch64/options_set_9.c
new file mode 100644
index 0000000000000000000000000000000000000000..e3c7cdc54ffb0616877260c562354496cfdcb688
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_9.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8-a+simd+fp" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv8\-a} 1 } } */
+
+ /* Check that grouping of bits that don't form a synthetic group don't turn
+    on the parent. e.g. rdma turns on simd+fp, but simd+fp does not turn on
+    rdma since rdma is it's own group.  crypto however turns on aes and sha2
+    and turning on sha2 and eas should turn on crypto!.  */

Reply via email to