AArch64: Fix command line options canonicalization version gcc-mirror#2. (PR target/88530)

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.

gcc/ChangeLog:

	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:

	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.



git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@269193 138bc75d-0d04-0410-961f-82ee72b054a4

Author: tnfchris

gcc/ChangeLog

@@ -1,3 +1,25 @@
+2019-02-25  Tamar Christina  <[email protected]>
+
+	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.
+
 2019-02-25  Tamar Christina  <[email protected]>
 
 	* config/arm/arm_neon.h (vfmlal_low_u32, vfmlsl_low_u32,

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,24 +166,35 @@ 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
 {
   const char *const name;
   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