From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 28B7A384DED7 for ; Thu, 14 Dec 2023 19:37:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 28B7A384DED7 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 28B7A384DED7 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702582632; cv=none; b=Byw9RzaAkuIYXCxbxi6lsefmD7DZiZaDU6k30SRqqN/x4FWuZzgOGbW3S/iDVJQ2ssbxekD8IWcSyL0s1baPzeiZ5T+1m/quA2wIuRAjYwWuPzHvE96ZsxfCneISoriPXEs0G8+dTwnSCtzmzcqB+kGXItDpprARCZVFyWFghvk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702582632; c=relaxed/simple; bh=5N9rjcljzeCOD1W2HDuaUgLjMx1JMmt/vfvgh5134V4=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=rIh023QlLkIhQOOVyEZa9y7oE6XXkNuYMycccdtfI2KXfAoOfqel11Fb8+wYjg8A8hPBR3yB40ggeJLO2NDQbxbnGe9Q4BXb3DZXbUFQ+CmNeUmsizsDPQI1gQz44xe3dHGozNpLsgP90v6meenUb9tn1lXP9SHuhSL4aZ5wGF0= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 522AFC15; Thu, 14 Dec 2023 11:37:56 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 60A183F738; Thu, 14 Dec 2023 11:37:10 -0800 (PST) From: Richard Sandiford To: Andrew Carlotti Mail-Followup-To: Andrew Carlotti ,gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH v2] aarch64: Fix +nopredres, +nols64 and +nomops References: Date: Thu, 14 Dec 2023 19:37:09 +0000 In-Reply-To: (Andrew Carlotti's message of "Wed, 13 Dec 2023 14:58:45 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-21.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_LOTSOFHASH,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Andrew Carlotti writes: > On Sat, Dec 09, 2023 at 07:22:49PM +0000, Richard Sandiford wrote: >> Andrew Carlotti writes: >> > ... >> >> This is the only use of native_detect_p, so it'd be good to remove >> the field itself. > > Done > >> > ... >> > >> > @@ -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. > > I added it because I realised that the parsing behaviour didn't make sense in > that case, and my patch happens to change the behaviour as well (the outcome > without the check would be no enabled features, whereas previously it would > enable only the features with no native detection). Ah, OK, thanks for the explanation. > I agree that it makes sense to put it with the original check, so I've made that change. > >> Looks good otherwise, thanks. >> >> Richard > > New patch version below, ok for master? > > --- > > 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 > (struct aarch64_option_extension): Remove unused field. > (all_extensions): Ditto. > (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_28.c: New test. OK, thanks. Richard > diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc > index c2a6d357c0bc17996a25ea5c3a40f69d745c7931..4d0431d3a2cad5414790646bce0c09877c0366b2 100644 > --- a/gcc/common/config/aarch64/aarch64-common.cc > +++ b/gcc/common/config/aarch64/aarch64-common.cc > @@ -149,9 +149,6 @@ struct aarch64_option_extension > aarch64_feature_flags flags_on; > /* If this feature is turned off, these bits also need to be turned off. */ > aarch64_feature_flags flags_off; > - /* Indicates whether this feature is taken into account during native cpu > - detection. */ > - bool native_detect_p; > }; > > /* ISA extensions in AArch64. */ > @@ -159,10 +156,9 @@ static constexpr aarch64_option_extension all_extensions[] = > { > #define AARCH64_OPT_EXTENSION(NAME, IDENT, C, D, E, FEATURE_STRING) \ > {NAME, AARCH64_FL_##IDENT, feature_deps::IDENT ().explicit_on, \ > - feature_deps::get_flags_off (feature_deps::root_off_##IDENT), \ > - FEATURE_STRING[0]}, > + feature_deps::get_flags_off (feature_deps::root_off_##IDENT)}, > #include "config/aarch64/aarch64-option-extensions.def" > - {NULL, 0, 0, 0, false} > + {NULL, 0, 0, 0} > }; > > struct processor_name_to_arch > @@ -358,8 +354,7 @@ aarch64_get_extension_string_for_isa_flags > /* If either crypto flag needs removing here, then both do. */ > flags = flags_crypto; > > - if (opt.native_detect_p > - && (flags & current_flags & ~isa_flags)) > + if (flags & current_flags & ~isa_flags) > { > current_flags &= ~opt.flags_off; > outstr += "+no"; > diff --git a/gcc/config/aarch64/driver-aarch64.cc b/gcc/config/aarch64/driver-aarch64.cc > index 8e318892b10aa2288421fad418844744a2f5a3b4..c18f065aa41e7328d71b45a53c82a3b703ae44d5 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; > > @@ -380,7 +384,8 @@ host_detect_local_cpu (int argc, const char **argv) > if (n_cores == 0 > || n_cores > 2 > || (n_cores == 1 && n_variants != 1) > - || imp == INVALID_IMP) > + || imp == INVALID_IMP > + || !processed_exts) > goto not_found; > > /* Simple case, one core type or just looking for the arch. */ > @@ -447,6 +452,10 @@ host_detect_local_cpu (int argc, const char **argv) > if (tune) > return res; > > + /* 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_28.c b/gcc/testsuite/gcc.target/aarch64/options_set_28.c > new file mode 100644 > index 0000000000000000000000000000000000000000..9e63768581e9d429e9408863942051b1b04761ac > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/options_set_28.c > @@ -0,0 +1,9 @@ > +/* { 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 } } */