On 14/09/2020 15:19, Wilco Dijkstra wrote:
> The --with-cpu/--with-arch configure option processing not only checks valid
> arguments
> but also sets TARGET_CPU_DEFAULT with a CPU and extension bitmask. This
> isn't used
> however since a --with-cpu is translated into a -mcpu option which is
> processed as if
> written on the command-line (so TARGET_CPU_DEFAULT is never accessed).
>
> So remove all the complex processing and bitmask, and just validate the
> option.
> Fix a bug that always reports valid architecture extensions as invalid. As a
> result
> the CPU processing in aarch64.c can be simplified.
>
> Bootstrap OK, regress pass, OK for commit?
Doesn't this change the default behaviour if cc1 is run directly? I'm
not saying this is the wrong thing to do (I think we rely on this in the
arm port), but I just want to understand by what you mean when you say
'never used'.
R.
>
> ChangeLog:
> 2020-09-03 Wilco Dijkstra <wdijk...@arm.com>
>
> * config.gcc (aarch64*-*-*): Simplify --with-cpu and --with-arch
> processing. Add support for architectural extensions.
> * config/aarch64/aarch64.h (TARGET_CPU_DEFAULT): Remove
> AARCH64_CPU_DEFAULT_FLAGS.
> * config/aarch64/aarch64.c (AARCH64_CPU_DEFAULT_FLAGS): Remove define.
> (get_tune_cpu): Assert CPU is always valid.
> (get_arch): Assert architecture is always valid.
> (aarch64_override_options): Cleanup CPU selection code and simplify
> logic.
>
> ---
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index
> 88e428fd2ad0fb605a53f5dfa58d9f14603b4302..918320573ade712ddc252045e0b70fb8b65e0c66
> 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4067,8 +4067,6 @@ case "${target}" in
> pattern=AARCH64_CORE
> fi
>
> - ext_mask=AARCH64_CPU_DEFAULT_FLAGS
> -
> # Find the base CPU or ARCH id in aarch64-cores.def or
> # aarch64-arches.def
> if [ x"$base_val" = x ] \
> @@ -4076,23 +4074,6 @@ case "${target}" in
> ${srcdir}/config/aarch64/$def \
> > /dev/null; then
>
> - if [ $which = arch ]; then
> - base_id=`grep "^$pattern(\"$base_val\"," \
> - ${srcdir}/config/aarch64/$def | \
> - sed -e 's/^[^,]*,[ ]*//' | \
> - sed -e 's/,.*$//'`
> - # Extract the architecture flags from
> aarch64-arches.def
> - ext_mask=`grep "^$pattern(\"$base_val\"," \
> - ${srcdir}/config/aarch64/$def | \
> - sed -e 's/)$//' | \
> - sed -e 's/^.*,//'`
> - else
> - base_id=`grep "^$pattern(\"$base_val\"," \
> - ${srcdir}/config/aarch64/$def | \
> - sed -e 's/^[^,]*,[ ]*//' | \
> - sed -e 's/,.*$//'`
> - fi
> -
> # Use the pre-processor to strip flatten the options.
> # This makes the format less rigid than if we use
> # grep and sed directly here.
> @@ -4117,25 +4098,7 @@ case "${target}" in
> grep "^\"$base_ext\""`
>
> if [ x"$base_ext" = x ] \
> - || [[ -n $opt_line ]]; then
> -
> - # These regexp extract the elements based on
> - # their group match index in the regexp.
> - ext_canon=`echo -e "$opt_line" | \
> - sed -e "s/$sed_patt/\2/"`
> - ext_on=`echo -e "$opt_line" | \
> - sed -e "s/$sed_patt/\3/"`
> - ext_off=`echo -e "$opt_line" | \
> - sed -e "s/$sed_patt/\4/"`
> -
> - if [ $ext = $base_ext ]; then
> - # Adding extension
> - ext_mask="("$ext_mask") | ("$ext_on" |
> "$ext_canon")"
> - else
> - # Removing extension
> - ext_mask="("$ext_mask") & ~("$ext_off"
> | "$ext_canon")"
> - fi
> -
> + || [ x"$opt_line" != x ]; then
> true
> else
> echo "Unknown extension used in
> --with-$which=$val" 1>&2
> @@ -4144,10 +4107,6 @@ case "${target}" in
> ext_val=`echo $ext_val | sed -e
> 's/[a-z0-9]\+//'`
> done
>
> - ext_mask="(("$ext_mask") << 6)"
> - if [ x"$base_id" != x ]; then
> - target_cpu_cname="TARGET_CPU_$base_id |
> $ext_mask"
> - fi
> true
> else
> echo "Unknown $which used in --with-$which=$val" 1>&2
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index
> d3e89d1789a23c2fa4d62744084adfdedac6a116..30b3a28a6d2893cc29bec4aa5b7cfbe1fd51e0b7
> 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -767,8 +767,7 @@ enum target_cpus
>
> /* If there is no CPU defined at configure, use generic as default. */
> #ifndef TARGET_CPU_DEFAULT
> -#define TARGET_CPU_DEFAULT \
> - (TARGET_CPU_generic | (AARCH64_CPU_DEFAULT_FLAGS << 6))
> +# define TARGET_CPU_DEFAULT TARGET_CPU_generic
> #endif
>
> /* If inserting NOP before a mult-accumulate insn remember to adjust the
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 24b455fd627bca1bdd7d81d5933aa46275061d7c..803562df25751f2eb6dbe18b67cae46ea7c478dd
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -1472,8 +1472,6 @@ static const struct attribute_spec
> aarch64_attribute_table[] =
> { NULL, 0, 0, false, false, false, false, NULL, NULL }
> };
>
> -#define AARCH64_CPU_DEFAULT_FLAGS ((selected_cpu) ? selected_cpu->flags : 0)
> -
> /* An ISA extension in the co-processor and main instruction set space. */
> struct aarch64_option_extension
> {
> @@ -14824,35 +14822,24 @@ aarch64_validate_mtune (const char *str, const
> struct processor **res)
> return false;
> }
>
> -/* Return the CPU corresponding to the enum CPU.
> - If it doesn't specify a cpu, return the default. */
> +/* Return the CPU corresponding to the enum CPU. */
>
> static const struct processor *
> aarch64_get_tune_cpu (enum aarch64_processor cpu)
> {
> - if (cpu != aarch64_none)
> - return &all_cores[cpu];
> + gcc_assert (cpu != aarch64_none);
>
> - /* The & 0x3f is to extract the bottom 6 bits that encode the
> - default cpu as selected by the --with-cpu GCC configure option
> - in config.gcc.
> - ???: The whole TARGET_CPU_DEFAULT and AARCH64_CPU_DEFAULT_FLAGS
> - flags mechanism should be reworked to make it more sane. */
> - return &all_cores[TARGET_CPU_DEFAULT & 0x3f];
> + return &all_cores[cpu];
> }
>
> -/* Return the architecture corresponding to the enum ARCH.
> - If it doesn't specify a valid architecture, return the default. */
> +/* Return the architecture corresponding to the enum ARCH. */
>
> static const struct processor *
> aarch64_get_arch (enum aarch64_arch arch)
> {
> - if (arch != aarch64_no_arch)
> - return &all_architectures[arch];
> -
> - const struct processor *cpu = &all_cores[TARGET_CPU_DEFAULT & 0x3f];
> + gcc_assert (arch != aarch64_no_arch);
>
> - return &all_architectures[cpu->arch];
> + return &all_architectures[arch];
> }
>
> /* Return the VG value associated with -msve-vector-bits= value VALUE. */
> @@ -14890,10 +14877,6 @@ aarch64_override_options (void)
> uint64_t arch_isa = 0;
> aarch64_isa_flags = 0;
>
> - bool valid_cpu = true;
> - bool valid_tune = true;
> - bool valid_arch = true;
> -
> selected_cpu = NULL;
> selected_arch = NULL;
> selected_tune = NULL;
> @@ -14908,77 +14891,56 @@ aarch64_override_options (void)
> If either of -march or -mtune is given, they override their
> respective component of -mcpu. */
> if (aarch64_cpu_string)
> - valid_cpu = aarch64_validate_mcpu (aarch64_cpu_string, &selected_cpu,
> - &cpu_isa);
> + aarch64_validate_mcpu (aarch64_cpu_string, &selected_cpu, &cpu_isa);
>
> if (aarch64_arch_string)
> - valid_arch = aarch64_validate_march (aarch64_arch_string, &selected_arch,
> - &arch_isa);
> + aarch64_validate_march (aarch64_arch_string, &selected_arch, &arch_isa);
>
> if (aarch64_tune_string)
> - valid_tune = aarch64_validate_mtune (aarch64_tune_string,
> &selected_tune);
> + aarch64_validate_mtune (aarch64_tune_string, &selected_tune);
>
> #ifdef SUBTARGET_OVERRIDE_OPTIONS
> SUBTARGET_OVERRIDE_OPTIONS;
> #endif
>
> - /* If the user did not specify a processor, choose the default
> - one for them. This will be the CPU set during configuration using
> - --with-cpu, otherwise it is "generic". */
> - if (!selected_cpu)
> - {
> - if (selected_arch)
> - {
> - selected_cpu = &all_cores[selected_arch->ident];
> - aarch64_isa_flags = arch_isa;
> - explicit_arch = selected_arch->arch;
> - }
> - else
> - {
> - /* Get default configure-time CPU. */
> - selected_cpu = aarch64_get_tune_cpu (aarch64_none);
> - aarch64_isa_flags = TARGET_CPU_DEFAULT >> 6;
> - }
> -
> - if (selected_tune)
> - explicit_tune_core = selected_tune->ident;
> - }
> - /* If both -mcpu and -march are specified check that they are
> architecturally
> - compatible, warn if they're not and prefer the -march ISA flags. */
> - else if (selected_arch)
> + if (selected_cpu && selected_arch)
> {
> + /* If both -mcpu and -march are specified, warn if they are not
> + architecturally compatible and prefer the -march ISA flags. */
> if (selected_arch->arch != selected_cpu->arch)
> {
> warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch",
> aarch64_cpu_string,
> aarch64_arch_string);
> }
> +
> aarch64_isa_flags = arch_isa;
> - explicit_arch = selected_arch->arch;
> - explicit_tune_core = selected_tune ? selected_tune->ident
> - : selected_cpu->ident;
> }
> - else
> + else if (selected_cpu)
> {
> - /* -mcpu but no -march. */
> - aarch64_isa_flags = cpu_isa;
> - explicit_tune_core = selected_tune ? selected_tune->ident
> - : selected_cpu->ident;
> - gcc_assert (selected_cpu);
> selected_arch = &all_architectures[selected_cpu->arch];
> - explicit_arch = selected_arch->arch;
> + aarch64_isa_flags = cpu_isa;
> }
> -
> - /* Set the arch as well as we will need it when outputing
> - the .arch directive in assembly. */
> - if (!selected_arch)
> + else if (selected_arch)
> {
> - gcc_assert (selected_cpu);
> + selected_cpu = &all_cores[selected_arch->ident];
> + aarch64_isa_flags = arch_isa;
> + }
> + else
> + {
> + /* No -mcpu or -march specified, so use the default CPU. */
> + selected_cpu = &all_cores[TARGET_CPU_DEFAULT];
> selected_arch = &all_architectures[selected_cpu->arch];
> + aarch64_isa_flags = selected_cpu->flags;
> }
>
> + explicit_arch = selected_arch->arch;
> if (!selected_tune)
> selected_tune = selected_cpu;
> + explicit_tune_core = selected_tune->ident;
> +
> + gcc_assert (explicit_tune_core != aarch64_none);
> + gcc_assert (explicit_arch != aarch64_no_arch);
>
> if (aarch64_enable_bti == 2)
> {
> @@ -15014,15 +14976,6 @@ aarch64_override_options (void)
> if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE && TARGET_ILP32)
> sorry ("return address signing is only supported for %<-mabi=lp64%>");
>
> - /* Make sure we properly set up the explicit options. */
> - if ((aarch64_cpu_string && valid_cpu)
> - || (aarch64_tune_string && valid_tune))
> - gcc_assert (explicit_tune_core != aarch64_none);
> -
> - if ((aarch64_cpu_string && valid_cpu)
> - || (aarch64_arch_string && valid_arch))
> - gcc_assert (explicit_arch != aarch64_no_arch);
> -
> /* The pass to insert speculation tracking runs before
> shrink-wrapping and the latter does not know how to update the
> tracking status. So disable it in this case. */
>
>
