Andrew Carlotti <andrew.carlo...@arm.com> writes: > Additionally, replace all checks for the AARCH64_FL_CRYPTO bit with > checks for (AARCH64_FL_AES | AARCH64_FL_SHA2) instead. The value of the > AARCH64_FL_CRYPTO bit within isa_flags is now ignored, but it is > retained because removing it would make processing the data in > option-extensions.def significantly more complex. > > This bug should have been picked up by an existing test, but a missing > newline meant that the pattern incorrectly allowed "+crypto+nocrypto". > > Ok for master? > > gcc/ChangeLog: > > * common/config/aarch64/aarch64-common.cc > (aarch64_get_extension_string_for_isa_flags): Fix generation of > the "+nocrypto" extension. > * config/aarch64/aarch64.h (AARCH64_ISA_CRYPTO): Remove. > (TARGET_CRYPTO): Remove. > * config/aarch64/aarch64-c.cc (aarch64_update_cpp_builtins): > Don't use TARGET_CRYPTO. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/options_set_4.c: Add terminating newline. > * gcc.target/aarch64/options_set_27.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 > 8fb901029ec2980a048177586b84201b3b398f9e..c2a6d357c0bc17996a25ea5c3a40f69d745c7931 > 100644 > --- a/gcc/common/config/aarch64/aarch64-common.cc > +++ b/gcc/common/config/aarch64/aarch64-common.cc > @@ -311,6 +311,7 @@ aarch64_get_extension_string_for_isa_flags > But in order to make the output more readable, it seems better > to add the strings in definition order. */ > aarch64_feature_flags added = 0; > + auto flags_crypto = AARCH64_FL_AES | AARCH64_FL_SHA2; > for (unsigned int i = ARRAY_SIZE (all_extensions); i-- > 0; ) > { > auto &opt = all_extensions[i]; > @@ -320,7 +321,7 @@ aarch64_get_extension_string_for_isa_flags > per-feature crypto flags. */ > auto flags = opt.flag_canonical; > if (flags == AARCH64_FL_CRYPTO) > - flags = AARCH64_FL_AES | AARCH64_FL_SHA2; > + flags = flags_crypto; > > if ((flags & isa_flags & (explicit_flags | ~current_flags)) == flags) > { > @@ -339,14 +340,32 @@ aarch64_get_extension_string_for_isa_flags > not have an HWCAPs then it shouldn't be taken into account for feature > detection because one way or another we can't tell if it's available > or not. */ > + > for (auto &opt : all_extensions) > - if (opt.native_detect_p > - && (opt.flag_canonical & current_flags & ~isa_flags)) > - { > - current_flags &= ~opt.flags_off; > - outstr += "+no"; > - outstr += opt.name; > - } > + { > + auto flags = opt.flag_canonical; > + /* As a special case, don't emit "+noaes" or "+nosha2" when we could > emit > + "+nocrypto" instead, in order to support assemblers that predate the > + separate per-feature crypto flags. Only allow "+nocrypto" when "sm4" > + is not already enabled (to avoid dependending on whether "+nocrypto" > + also disables "sm4"). */ > + if (flags & flags_crypto > + && (flags_crypto & current_flags & ~isa_flags) == flags_crypto > + && !(current_flags & AARCH64_FL_SM4)) > + continue; > + > + if (flags == AARCH64_FL_CRYPTO) > + /* If either crypto flag needs removing here, then both do. */ > + flags = flags_crypto; > + > + if (opt.native_detect_p > + && (flags & current_flags & ~isa_flags)) > + { > + current_flags &= ~opt.flags_off; > + outstr += "+no"; > + outstr += opt.name; > + } > + } > > return outstr; > } > diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc > index > 115a2a8b7568c43a712d819e03147ff84ff182c0..cdc4e453a2054b1a1d2c70bf0b528e497ae0b9ad > 100644 > --- a/gcc/config/aarch64/aarch64-c.cc > +++ b/gcc/config/aarch64/aarch64-c.cc > @@ -188,7 +188,7 @@ aarch64_update_cpp_builtins (cpp_reader *pfile) > aarch64_def_or_undef (TARGET_ILP32, "_ILP32", pfile); > aarch64_def_or_undef (TARGET_ILP32, "__ILP32__", pfile); > > - aarch64_def_or_undef (TARGET_CRYPTO, "__ARM_FEATURE_CRYPTO", pfile); > + aarch64_def_or_undef (TARGET_AES && TARGET_SHA2, "__ARM_FEATURE_CRYPTO", > pfile); > aarch64_def_or_undef (TARGET_SIMD_RDMA, "__ARM_FEATURE_QRDMX", pfile); > aarch64_def_or_undef (TARGET_SVE, "__ARM_FEATURE_SVE", pfile); > cpp_undef (pfile, "__ARM_FEATURE_SVE_BITS"); > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h > index > 2cd0bc552ebadac06a2838ae2767852c036d0db4..501bb7478a0755fa76c488ec03dcfab6c272851c > 100644 > --- a/gcc/config/aarch64/aarch64.h > +++ b/gcc/config/aarch64/aarch64.h > @@ -204,7 +204,11 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = > AARCH64_FL_SM_OFF; > > #endif > > -/* Macros to test ISA flags. */ > +/* Macros to test ISA flags. > + > + There is intentionally no macro for AARCH64_FL_CRYPTO, since this flag bit > + is not always set when its constituent features are present. > + Check (TARGET_AES && TARGET_SHA2) instead. */ > > #define AARCH64_ISA_SM_OFF (aarch64_isa_flags & AARCH64_FL_SM_OFF) > #define AARCH64_ISA_SM_ON (aarch64_isa_flags & AARCH64_FL_SM_ON) > @@ -213,7 +217,6 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = > AARCH64_FL_SM_OFF; > #define AARCH64_ISA_V8A (aarch64_isa_flags & AARCH64_FL_V8A) > #define AARCH64_ISA_V8_1A (aarch64_isa_flags & AARCH64_FL_V8_1A) > #define AARCH64_ISA_CRC (aarch64_isa_flags & AARCH64_FL_CRC) > -#define AARCH64_ISA_CRYPTO (aarch64_isa_flags & AARCH64_FL_CRYPTO) > #define AARCH64_ISA_FP (aarch64_isa_flags & AARCH64_FL_FP) > #define AARCH64_ISA_SIMD (aarch64_isa_flags & AARCH64_FL_SIMD) > #define AARCH64_ISA_LSE (aarch64_isa_flags & AARCH64_FL_LSE) > @@ -294,9 +297,6 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = > AARCH64_FL_SM_OFF; > #define AARCH64_FL_SCXTNUM AARCH64_FL_V8_5A > #define AARCH64_FL_ID_PFR2 AARCH64_FL_V8_5A > > -/* Crypto is an optional extension to AdvSIMD. */ > -#define TARGET_CRYPTO (AARCH64_ISA_CRYPTO) > - > /* SHA2 is an optional extension to AdvSIMD. */ > #define TARGET_SHA2 (AARCH64_ISA_SHA2) > > diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_27.c > b/gcc/testsuite/gcc.target/aarch64/options_set_27.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..e35744640130778cba442c365e70028e53508586 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/options_set_27.c > @@ -0,0 +1,9 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-march=armv8.2-a+aes+sha3" } */ > + > +int main () > +{ > + return 0; > +} > + > +/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crc\+aes\+sha3\n} > 1 } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_4.c > b/gcc/testsuite/gcc.target/aarch64/options_set_4.c > index > 5370e02e1531dd26e5a5fb37de0bab6aed513b71..7b00d09a47f0521f148b1dd576b4100db38aec00 > 100644 > --- a/gcc/testsuite/gcc.target/aarch64/options_set_4.c > +++ b/gcc/testsuite/gcc.target/aarch64/options_set_4.c > @@ -6,7 +6,7 @@ int main () > return 0; > } > > -/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crc\+crypto} 1 } } > */ > +/* { dg-final { scan-assembler-times {\.arch armv8\.2\-a\+crc\+crypto\n} 1 } > } */ > > /* Check if individual bits that make up a grouping is specified that only > the > grouping is kept. */