Andrew Carlotti <andrew.carlo...@arm.com> writes:
> For native cpu feature detection, certain features have no entry in
> /proc/cpuinfo, so have to be assumed to be present whenever the detected
> cpu is supposed to support that feature.
>
> However, the logic for this was mistakenly implemented by excluding
> these features from part of aarch64_get_extension_string_for_isa_flags.
> This function is also used elsewhere when canonicalising explicit
> feature sets, which may require removing features that are normally
> implied by the specified architecture version.
>
> This change reenables generation of +nopredres, +nols64 and +nomops
> during canonicalisation, by relocating the misplaced native cpu
> detection logic.
>
> gcc/ChangeLog:
>
> * common/config/aarch64/aarch64-common.cc
> (aarch64_get_extension_string_for_isa_flags): Remove filtering
> of features without native detection.
> * config/aarch64/driver-aarch64.cc (host_detect_local_cpu):
> Explicitly add expected features that lack cpuinfo detection.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/options_set_29.c: New test.
>
>
> diff --git a/gcc/common/config/aarch64/aarch64-common.cc
> b/gcc/common/config/aarch64/aarch64-common.cc
> index
> ee2ea7eae105d19ec906ef8d25d3a237fbeac4b4..37e60d6083e290b18b1f4c6274123b0a58de5476
> 100644
> --- a/gcc/common/config/aarch64/aarch64-common.cc
> +++ b/gcc/common/config/aarch64/aarch64-common.cc
> @@ -357,8 +357,7 @@ aarch64_get_extension_string_for_isa_flags
> }
>
> for (auto &opt : all_extensions)
> - if (opt.native_detect_p
> - && (opt.flag_canonical != AARCH64_FL_CRYPTO)
> + if ((opt.flag_canonical != AARCH64_FL_CRYPTO)
> && (opt.flag_canonical & current_flags & ~isa_flags))
> {
> current_flags &= ~opt.flags_off;
This is the only use of native_detect_p, so it'd be good to remove
the field itself.
> diff --git a/gcc/config/aarch64/driver-aarch64.cc
> b/gcc/config/aarch64/driver-aarch64.cc
> index
> 8e318892b10aa2288421fad418844744a2f5a3b4..470c19b650f1ae953918eaeddbf0f768c12a99d9
> 100644
> --- a/gcc/config/aarch64/driver-aarch64.cc
> +++ b/gcc/config/aarch64/driver-aarch64.cc
> @@ -262,6 +262,7 @@ host_detect_local_cpu (int argc, const char **argv)
> unsigned int n_variants = 0;
> bool processed_exts = false;
> aarch64_feature_flags extension_flags = 0;
> + aarch64_feature_flags unchecked_extension_flags = 0;
> aarch64_feature_flags default_flags = 0;
> std::string buf;
> size_t sep_pos = -1;
> @@ -348,7 +349,10 @@ host_detect_local_cpu (int argc, const char **argv)
> /* If the feature contains no HWCAPS string then ignore it for the
> auto detection. */
> if (val.empty ())
> - continue;
> + {
> + unchecked_extension_flags |= aarch64_extensions[i].flag;
> + continue;
> + }
>
> bool enabled = true;
>
> @@ -447,6 +451,13 @@ host_detect_local_cpu (int argc, const char **argv)
> if (tune)
> return res;
>
> + if (!processed_exts)
> + goto not_found;
Could you explain this part? It seems like more of a parsing change
(i.e. being more strict about what we accept).
If that's the intention, it probably belongs in:
if (n_cores == 0
|| n_cores > 2
|| (n_cores == 1 && n_variants != 1)
|| imp == INVALID_IMP)
goto not_found;
But maybe it should be a separate patch.
Looks good otherwise, thanks.
Richard
> +
> + /* Add any features that should be be present, but can't be verified using
> + the /proc/cpuinfo "Features" list. */
> + extension_flags |= unchecked_extension_flags & default_flags;
> +
> {
> std::string extension
> = aarch64_get_extension_string_for_isa_flags (extension_flags,
> diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_29.c
> b/gcc/testsuite/gcc.target/aarch64/options_set_29.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..01bb73c02e232bdfeca5f16dad3fa2a6484843d5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/options_set_29.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-march=armv9.3-a+nopredres+nols64+nomops" } */
> +
> +int main ()
> +{
> + return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times {\.arch
> armv9\.3\-a\+crc\+nopredres\+nols64\+nomops\n} 1 } } */
> +
> +/* Checking if enabling default features drops the superfluous bits. */
