* [PATCH] AArch64: Cleanup CPU option processing code
@ 2022-05-09 16:36 Wilco Dijkstra
2022-05-09 17:24 ` Richard Sandiford
0 siblings, 1 reply; 6+ messages in thread
From: Wilco Dijkstra @ 2022-05-09 16:36 UTC (permalink / raw)
To: Kyrylo Tkachov, Richard Sandiford; +Cc: GCC Patches
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.cc can be simplified.
Bootstrap OK, regress pass, OK for commit?
ChangeLog:
2022-04-19 Wilco Dijkstra <wdijkstr@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.
(TARGET_CPU_NBITS): Remove.
(TARGET_CPU_MASK): Remove.
* config/aarch64/aarch64.cc (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 c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4178,8 +4178,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 ] \
@@ -4187,23 +4185,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
-
# Disallow extensions in --with-tune=cortex-a53+crc.
if [ $which = tune ] && [ x"$ext_val" != x ]; then
echo "Architecture extensions not supported in --with-$which=$val" 1>&2
@@ -4234,25 +4215,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
@@ -4261,10 +4224,6 @@ case "${target}" in
ext_val=`echo $ext_val | sed -e 's/[a-z0-9]\+//'`
done
- ext_mask="(("$ext_mask") << TARGET_CPU_NBITS)"
- if [ x"$base_id" != x ]; then
- target_cpu_cname="TARGET_CPU_$base_id | $ext_mask"
- fi
true
else
# Allow --with-$which=native.
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 359b6e8561faa38f53a806b8c114c83ae37e7e17..f835da33b72f36bbf25a0e1328135411bd8ab4f6 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -813,16 +813,9 @@ enum target_cpus
TARGET_CPU_generic
};
-/* Define how many bits are used to represent the CPU in TARGET_CPU_DEFAULT.
- This needs to be big enough to fit the value of TARGET_CPU_generic.
- All bits after this are used to represent the AARCH64_CPU_DEFAULT_FLAGS. */
-#define TARGET_CPU_NBITS 8
-#define TARGET_CPU_MASK ((1 << TARGET_CPU_NBITS) - 1)
-
/* 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 << TARGET_CPU_NBITS))
+# 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.cc b/gcc/config/aarch64/aarch64.cc
index f650abbc4ce49cf0947049931f86bad1130c3428..43d87d1b9c4ef1a85094e51f81745f98f1ef27fb 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -2760,8 +2760,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
{
@@ -18057,39 +18055,24 @@ aarch64_validate_mtune (const char *str, const struct processor **res)
return false;
}
-static_assert (TARGET_CPU_generic < TARGET_CPU_MASK,
- "TARGET_CPU_NBITS is big enough");
-
-/* 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 & TARGET_CPU_MASK is to extract the bottom TARGET_CPU_NBITS 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 & TARGET_CPU_MASK];
+ 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 & TARGET_CPU_MASK];
+ 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. */
@@ -18127,10 +18110,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;
@@ -18145,77 +18124,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 >> TARGET_CPU_NBITS;
- }
-
- 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)
{
@@ -18251,15 +18209,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. */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AArch64: Cleanup CPU option processing code
2022-05-09 16:36 [PATCH] AArch64: Cleanup CPU option processing code Wilco Dijkstra
@ 2022-05-09 17:24 ` Richard Sandiford
2022-05-11 14:44 ` Wilco Dijkstra
0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2022-05-09 17:24 UTC (permalink / raw)
To: Wilco Dijkstra via Gcc-patches; +Cc: Kyrylo Tkachov, Wilco Dijkstra
Wilco Dijkstra via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> 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.cc can be simplified.
>
> Bootstrap OK, regress pass, OK for commit?
Although invoking ./cc1 directly only half-works with --with-arch,
it half-works well-enough that I'd still like to keep it working.
But I agree we should apply your change first, then I can follow up
with a patch to make --with-* work with ./cc1 later. (I have a version
locally and the net result is much simpler than the status quo, as well
as hopefully actually working properly.)
My main question about the patch itself is:
> + 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);
Do we still need both selected_arch and explicit_arch? explicit_arch
seems a misnomer now, since it includes implicit as well as explicit
choices. Same for selected_tune and explicit_tune_core.
aarch64_option_restore has:
if (opts->x_explicit_tune_core == aarch64_none
&& opts->x_explicit_arch != aarch64_no_arch)
selected_tune = &all_cores[selected_arch->ident];
else
selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
Is the “if” condition ever true, or can we now restore the tune
info unconditionally?
Thanks,
Richard
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AArch64: Cleanup CPU option processing code
2022-05-09 17:24 ` Richard Sandiford
@ 2022-05-11 14:44 ` Wilco Dijkstra
2022-05-12 8:02 ` Richard Sandiford
0 siblings, 1 reply; 6+ messages in thread
From: Wilco Dijkstra @ 2022-05-11 14:44 UTC (permalink / raw)
To: Richard Sandiford; +Cc: Kyrylo Tkachov, Wilco Dijkstra via Gcc-patches
Hi Richard,
> Although invoking ./cc1 directly only half-works with --with-arch,
> it half-works well-enough that I'd still like to keep it working.
> But I agree we should apply your change first, then I can follow up
> with a patch to make --with-* work with ./cc1 later. (I have a version
> locally and the net result is much simpler than the status quo, as well
> as hopefully actually working properly.)
That sounds good indeed. Is that changing TARGET_CPU_DEFAULT into
a string can could just be parsed like a -mcpu option?
> Do we still need both selected_arch and explicit_arch? explicit_arch
> seems a misnomer now, since it includes implicit as well as explicit
> choices. Same for selected_tune and explicit_tune_core.
At the moment we do since these are settings that must be saved/restored
since they can be overridden. However it may be possible to do further
cleanups to remove some of this. I also wonder whether we can remove the
internal override feature (override_tune_string) since that further complicates
the tunings, and I'm not convinced that it is either useful or being used at all.
> aarch64_option_restore has:
>
> if (opts->x_explicit_tune_core == aarch64_none
> && opts->x_explicit_arch != aarch64_no_arch)
> selected_tune = &all_cores[selected_arch->ident];
> else
> selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
>
> Is the “if” condition ever true, or can we now restore the tune
> info unconditionally?
Yes that was added a year or so after I created this patch, so this
is now redundant. I've removed it in v2:
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?
ChangeLog:
2022-04-19 Wilco Dijkstra <wdijkstr@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.
(TARGET_CPU_NBITS): Remove.
(TARGET_CPU_MASK): Remove.
* config/aarch64/aarch64.cc (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.
(aarch64_option_restore): Remove unnecessary checks on tune.
---
diff --git a/gcc/config.gcc b/gcc/config.gcc
index c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4178,8 +4178,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 ] \
@@ -4187,23 +4185,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
-
# Disallow extensions in --with-tune=cortex-a53+crc.
if [ $which = tune ] && [ x"$ext_val" != x ]; then
echo "Architecture extensions not supported in --with-$which=$val" 1>&2
@@ -4234,25 +4215,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
@@ -4261,10 +4224,6 @@ case "${target}" in
ext_val=`echo $ext_val | sed -e 's/[a-z0-9]\+//'`
done
- ext_mask="(("$ext_mask") << TARGET_CPU_NBITS)"
- if [ x"$base_id" != x ]; then
- target_cpu_cname="TARGET_CPU_$base_id | $ext_mask"
- fi
true
else
# Allow --with-$which=native.
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 359b6e8561faa38f53a806b8c114c83ae37e7e17..f835da33b72f36bbf25a0e1328135411bd8ab4f6 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -813,16 +813,9 @@ enum target_cpus
TARGET_CPU_generic
};
-/* Define how many bits are used to represent the CPU in TARGET_CPU_DEFAULT.
- This needs to be big enough to fit the value of TARGET_CPU_generic.
- All bits after this are used to represent the AARCH64_CPU_DEFAULT_FLAGS. */
-#define TARGET_CPU_NBITS 8
-#define TARGET_CPU_MASK ((1 << TARGET_CPU_NBITS) - 1)
-
/* 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 << TARGET_CPU_NBITS))
+# 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.cc b/gcc/config/aarch64/aarch64.cc
index f650abbc4ce49cf0947049931f86bad1130c3428..43d87d1b9c4ef1a85094e51f81745f98f1ef27fb 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -2760,8 +2760,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
{
@@ -18057,39 +18055,24 @@ aarch64_validate_mtune (const char *str, const struct processor **res)
return false;
}
-static_assert (TARGET_CPU_generic < TARGET_CPU_MASK,
- "TARGET_CPU_NBITS is big enough");
-
-/* 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 & TARGET_CPU_MASK is to extract the bottom TARGET_CPU_NBITS 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 & TARGET_CPU_MASK];
+ 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 & TARGET_CPU_MASK];
+ 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. */
@@ -18127,10 +18110,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;
@@ -18145,77 +18124,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 >> TARGET_CPU_NBITS;
- }
-
- 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)
{
@@ -18251,15 +18209,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. */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AArch64: Cleanup CPU option processing code
2022-05-11 14:44 ` Wilco Dijkstra
@ 2022-05-12 8:02 ` Richard Sandiford
2022-05-12 14:38 ` Wilco Dijkstra
0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2022-05-12 8:02 UTC (permalink / raw)
To: Wilco Dijkstra via Gcc-patches; +Cc: Wilco Dijkstra
Wilco Dijkstra via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hi Richard,
>
>> Although invoking ./cc1 directly only half-works with --with-arch,
>> it half-works well-enough that I'd still like to keep it working.
>> But I agree we should apply your change first, then I can follow up
>> with a patch to make --with-* work with ./cc1 later. (I have a version
>> locally and the net result is much simpler than the status quo, as well
>> as hopefully actually working properly.)
>
> That sounds good indeed. Is that changing TARGET_CPU_DEFAULT into
> a string can could just be parsed like a -mcpu option?
Yeah, it emulates the DRIVER_SELF_SPECS stuff using string macros.
>> Do we still need both selected_arch and explicit_arch? explicit_arch
>> seems a misnomer now, since it includes implicit as well as explicit
>> choices. Same for selected_tune and explicit_tune_core.
>
> At the moment we do since these are settings that must be saved/restored
> since they can be overridden.
Right, but I was wondering if we could save/restore them based on the
selected_* values instead. However…
> However it may be possible to do further cleanups to remove some of this.
…I agree that might as well be a separate clean-up.
> I also wonder whether we can remove the internal override feature
> (override_tune_string) since that further complicates the tunings, and
> I'm not convinced that it is either useful or being used at all.
It's a developer option rather than a user-facing thing. I found it
really useful when doing the Neoverse V1 tuning, and there are some
tests that rely on it. I'd prefer to keep it if possible.
>> aarch64_option_restore has:
>>
>> if (opts->x_explicit_tune_core == aarch64_none
>> && opts->x_explicit_arch != aarch64_no_arch)
>> selected_tune = &all_cores[selected_arch->ident];
>> else
>> selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
>>
>> Is the “if” condition ever true, or can we now restore the tune
>> info unconditionally?
>
> Yes that was added a year or so after I created this patch, so this
> is now redundant. I've removed it in v2:
>
>
> 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?
>
> ChangeLog:
> 2022-04-19 Wilco Dijkstra <wdijkstr@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.
> (TARGET_CPU_NBITS): Remove.
> (TARGET_CPU_MASK): Remove.
> * config/aarch64/aarch64.cc (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.
> (aarch64_option_restore): Remove unnecessary checks on tune.
Looks like you might have attached the old patch. The aarch64_option_restore
change is mentioned in the changelog but doesn't appear in the patch itself.
Thanks,
Richard
>
> ---
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4178,8 +4178,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 ] \
> @@ -4187,23 +4185,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
> -
> # Disallow extensions in --with-tune=cortex-a53+crc.
> if [ $which = tune ] && [ x"$ext_val" != x ]; then
> echo "Architecture extensions not supported in --with-$which=$val" 1>&2
> @@ -4234,25 +4215,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
> @@ -4261,10 +4224,6 @@ case "${target}" in
> ext_val=`echo $ext_val | sed -e 's/[a-z0-9]\+//'`
> done
>
> - ext_mask="(("$ext_mask") << TARGET_CPU_NBITS)"
> - if [ x"$base_id" != x ]; then
> - target_cpu_cname="TARGET_CPU_$base_id | $ext_mask"
> - fi
> true
> else
> # Allow --with-$which=native.
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 359b6e8561faa38f53a806b8c114c83ae37e7e17..f835da33b72f36bbf25a0e1328135411bd8ab4f6 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -813,16 +813,9 @@ enum target_cpus
> TARGET_CPU_generic
> };
>
> -/* Define how many bits are used to represent the CPU in TARGET_CPU_DEFAULT.
> - This needs to be big enough to fit the value of TARGET_CPU_generic.
> - All bits after this are used to represent the AARCH64_CPU_DEFAULT_FLAGS. */
> -#define TARGET_CPU_NBITS 8
> -#define TARGET_CPU_MASK ((1 << TARGET_CPU_NBITS) - 1)
> -
> /* 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 << TARGET_CPU_NBITS))
> +# 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.cc b/gcc/config/aarch64/aarch64.cc
> index f650abbc4ce49cf0947049931f86bad1130c3428..43d87d1b9c4ef1a85094e51f81745f98f1ef27fb 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -2760,8 +2760,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
> {
> @@ -18057,39 +18055,24 @@ aarch64_validate_mtune (const char *str, const struct processor **res)
> return false;
> }
>
> -static_assert (TARGET_CPU_generic < TARGET_CPU_MASK,
> - "TARGET_CPU_NBITS is big enough");
> -
> -/* 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 & TARGET_CPU_MASK is to extract the bottom TARGET_CPU_NBITS 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 & TARGET_CPU_MASK];
> + 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 & TARGET_CPU_MASK];
> + 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. */
> @@ -18127,10 +18110,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;
> @@ -18145,77 +18124,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 >> TARGET_CPU_NBITS;
> - }
> -
> - 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)
> {
> @@ -18251,15 +18209,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. */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AArch64: Cleanup CPU option processing code
2022-05-12 8:02 ` Richard Sandiford
@ 2022-05-12 14:38 ` Wilco Dijkstra
2022-05-12 15:05 ` Richard Sandiford
0 siblings, 1 reply; 6+ messages in thread
From: Wilco Dijkstra @ 2022-05-12 14:38 UTC (permalink / raw)
To: Richard Sandiford; +Cc: Wilco Dijkstra via Gcc-patches
Hi Richard,
> Looks like you might have attached the old patch. The aarch64_option_restore
> change is mentioned in the changelog but doesn't appear in the patch itself.
Indeed, not sure how that happened. Here is the correct v2 anyway.
Wilco
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?
ChangeLog:
2022-04-19 Wilco Dijkstra <wdijkstr@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.
(TARGET_CPU_NBITS): Remove.
(TARGET_CPU_MASK): Remove.
* config/aarch64/aarch64.cc (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.
(aarch64_option_restore): Remove unnecessary checks on tune.
---
diff --git a/gcc/config.gcc b/gcc/config.gcc
index c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4178,8 +4178,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 ] \
@@ -4187,23 +4185,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
-
# Disallow extensions in --with-tune=cortex-a53+crc.
if [ $which = tune ] && [ x"$ext_val" != x ]; then
echo "Architecture extensions not supported in --with-$which=$val" 1>&2
@@ -4234,25 +4215,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
@@ -4261,10 +4224,6 @@ case "${target}" in
ext_val=`echo $ext_val | sed -e 's/[a-z0-9]\+//'`
done
- ext_mask="(("$ext_mask") << TARGET_CPU_NBITS)"
- if [ x"$base_id" != x ]; then
- target_cpu_cname="TARGET_CPU_$base_id | $ext_mask"
- fi
true
else
# Allow --with-$which=native.
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 359b6e8561faa38f53a806b8c114c83ae37e7e17..f835da33b72f36bbf25a0e1328135411bd8ab4f6 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -813,16 +813,9 @@ enum target_cpus
TARGET_CPU_generic
};
-/* Define how many bits are used to represent the CPU in TARGET_CPU_DEFAULT.
- This needs to be big enough to fit the value of TARGET_CPU_generic.
- All bits after this are used to represent the AARCH64_CPU_DEFAULT_FLAGS. */
-#define TARGET_CPU_NBITS 8
-#define TARGET_CPU_MASK ((1 << TARGET_CPU_NBITS) - 1)
-
/* 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 << TARGET_CPU_NBITS))
+# 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.cc b/gcc/config/aarch64/aarch64.cc
index 1ec15741a4dba055b02732985d5a92a9252b166b..9294de799461f7f94c563f56b02e6e485ab7f1e6 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -2760,8 +2760,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
{
@@ -18040,39 +18038,24 @@ aarch64_validate_mtune (const char *str, const struct processor **res)
return false;
}
-static_assert (TARGET_CPU_generic < TARGET_CPU_MASK,
- "TARGET_CPU_NBITS is big enough");
-
-/* 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 & TARGET_CPU_MASK is to extract the bottom TARGET_CPU_NBITS 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 & TARGET_CPU_MASK];
+ 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 & TARGET_CPU_MASK];
+ 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. */
@@ -18110,10 +18093,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;
@@ -18128,77 +18107,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 >> TARGET_CPU_NBITS;
- }
-
- 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)
{
@@ -18234,15 +18192,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. */
@@ -18348,11 +18297,7 @@ aarch64_option_restore (struct gcc_options *opts,
opts->x_explicit_arch = ptr->x_explicit_arch;
selected_arch = aarch64_get_arch (ptr->x_explicit_arch);
opts->x_explicit_tune_core = ptr->x_explicit_tune_core;
- if (opts->x_explicit_tune_core == aarch64_none
- && opts->x_explicit_arch != aarch64_no_arch)
- selected_tune = &all_cores[selected_arch->ident];
- else
- selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
+ selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
opts->x_aarch64_override_tune_string = ptr->x_aarch64_override_tune_string;
opts->x_aarch64_branch_protection_string
= ptr->x_aarch64_branch_protection_string;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AArch64: Cleanup CPU option processing code
2022-05-12 14:38 ` Wilco Dijkstra
@ 2022-05-12 15:05 ` Richard Sandiford
0 siblings, 0 replies; 6+ messages in thread
From: Richard Sandiford @ 2022-05-12 15:05 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: Wilco Dijkstra via Gcc-patches
Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Hi Richard,
>
>> Looks like you might have attached the old patch. The aarch64_option_restore
>> change is mentioned in the changelog but doesn't appear in the patch itself.
>
> Indeed, not sure how that happened. Here is the correct v2 anyway.
>
> Wilco
>
>
> 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?
>
> ChangeLog:
> 2022-04-19 Wilco Dijkstra <wdijkstr@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.
> (TARGET_CPU_NBITS): Remove.
> (TARGET_CPU_MASK): Remove.
> * config/aarch64/aarch64.cc (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.
> (aarch64_option_restore): Remove unnecessary checks on tune.
OK, thanks.
Richard
> ---
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index c5064dd376660c192d5573997b4fc86b6b3e3838..b48d5451e8027c93fb1f614812589183d0a88c4b 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4178,8 +4178,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 ] \
> @@ -4187,23 +4185,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
> -
> # Disallow extensions in --with-tune=cortex-a53+crc.
> if [ $which = tune ] && [ x"$ext_val" != x ]; then
> echo "Architecture extensions not supported in --with-$which=$val" 1>&2
> @@ -4234,25 +4215,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
> @@ -4261,10 +4224,6 @@ case "${target}" in
> ext_val=`echo $ext_val | sed -e 's/[a-z0-9]\+//'`
> done
>
> - ext_mask="(("$ext_mask") << TARGET_CPU_NBITS)"
> - if [ x"$base_id" != x ]; then
> - target_cpu_cname="TARGET_CPU_$base_id | $ext_mask"
> - fi
> true
> else
> # Allow --with-$which=native.
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 359b6e8561faa38f53a806b8c114c83ae37e7e17..f835da33b72f36bbf25a0e1328135411bd8ab4f6 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -813,16 +813,9 @@ enum target_cpus
> TARGET_CPU_generic
> };
>
> -/* Define how many bits are used to represent the CPU in TARGET_CPU_DEFAULT.
> - This needs to be big enough to fit the value of TARGET_CPU_generic.
> - All bits after this are used to represent the AARCH64_CPU_DEFAULT_FLAGS. */
> -#define TARGET_CPU_NBITS 8
> -#define TARGET_CPU_MASK ((1 << TARGET_CPU_NBITS) - 1)
> -
> /* 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 << TARGET_CPU_NBITS))
> +# 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.cc b/gcc/config/aarch64/aarch64.cc
> index 1ec15741a4dba055b02732985d5a92a9252b166b..9294de799461f7f94c563f56b02e6e485ab7f1e6 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -2760,8 +2760,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
> {
> @@ -18040,39 +18038,24 @@ aarch64_validate_mtune (const char *str, const struct processor **res)
> return false;
> }
>
> -static_assert (TARGET_CPU_generic < TARGET_CPU_MASK,
> - "TARGET_CPU_NBITS is big enough");
> -
> -/* 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 & TARGET_CPU_MASK is to extract the bottom TARGET_CPU_NBITS 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 & TARGET_CPU_MASK];
> + 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 & TARGET_CPU_MASK];
> + 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. */
> @@ -18110,10 +18093,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;
> @@ -18128,77 +18107,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 >> TARGET_CPU_NBITS;
> - }
> -
> - 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)
> {
> @@ -18234,15 +18192,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. */
> @@ -18348,11 +18297,7 @@ aarch64_option_restore (struct gcc_options *opts,
> opts->x_explicit_arch = ptr->x_explicit_arch;
> selected_arch = aarch64_get_arch (ptr->x_explicit_arch);
> opts->x_explicit_tune_core = ptr->x_explicit_tune_core;
> - if (opts->x_explicit_tune_core == aarch64_none
> - && opts->x_explicit_arch != aarch64_no_arch)
> - selected_tune = &all_cores[selected_arch->ident];
> - else
> - selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
> + selected_tune = aarch64_get_tune_cpu (ptr->x_explicit_tune_core);
> opts->x_aarch64_override_tune_string = ptr->x_aarch64_override_tune_string;
> opts->x_aarch64_branch_protection_string
> = ptr->x_aarch64_branch_protection_string;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-12 15:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09 16:36 [PATCH] AArch64: Cleanup CPU option processing code Wilco Dijkstra
2022-05-09 17:24 ` Richard Sandiford
2022-05-11 14:44 ` Wilco Dijkstra
2022-05-12 8:02 ` Richard Sandiford
2022-05-12 14:38 ` Wilco Dijkstra
2022-05-12 15:05 ` Richard Sandiford
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).