From 154ae7d4e921d704118d6a16ee5cc2a10b9047da Mon Sep 17 00:00:00 2001 From: Fei Yang Date: Wed, 22 Apr 2020 18:24:59 +0100 Subject: [PATCH] aarch64: unexpected result with -mgeneral-regs-only and sve [PR94678] As the two testcases for PR94678 show, -mgeneral-regs-only is handled properly with SVE. We should issue an error message instead of expanding SVE builtin funtions when -mgeneral-regs-only option is specified. The middle end should never try to use vector patterns when the vector modes have been disabled by !have_regs_of_mode. But it's still wrong for the target to provide patterns that would inevitably lead to spill failure due to lack of registers. So we should also add check for !TARGET_GENERAL_REGS_ONLY in TARGET_SVE and other SVE related macros. 2020-04-22 Felix Yang gcc/ PR target/94678 * config/aarch64/aarch64.h (TARGET_SVE): Add && !TARGET_GENERAL_REGS_ONLY. (TARGET_SVE2): Add && TARGET_SVE. (TARGET_SVE2_AES, TARGET_SVE2_BITPERM, TARGET_SVE2_SHA3, TARGET_SVE2_SM4): Add && TARGET_SVE2. * config/aarch64/aarch64-sve-builtins.h (sve_switcher::m_old_general_regs_only): New member. * config/aarch64/aarch64-sve-builtins.cc (check_required_registers): New function. (reported_missing_registers_p): New variable. (check_required_extensions): Call check_required_registers before return if all required extenstions are present. (sve_switcher::sve_switcher): Save TARGET_GENERAL_REGS_ONLY in m_old_general_regs_only and clear MASK_GENERAL_REGS_ONLY in global_options.x_target_flags. (sve_switcher::~sve_switcher): Set MASK_GENERAL_REGS_ONLY in global_options.x_target_flags if m_old_general_regs_only is true. gcc/testsuite/ PR target/94678 * gcc.target/aarch64/sve/acle/general/nosve_6.c: New test. --- gcc/ChangeLog | 21 ++++++++++++ gcc/config/aarch64/aarch64-sve-builtins.cc | 34 ++++++++++++++++++- gcc/config/aarch64/aarch64-sve-builtins.h | 1 + gcc/config/aarch64/aarch64.h | 12 +++---- gcc/testsuite/ChangeLog | 5 +++ .../aarch64/sve/acle/general/nosve_6.c | 12 +++++++ 6 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 3b9af1ca346..085135c7a08 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,24 @@ +2020-04-22 Felix Yang + + PR target/94678 + * config/aarch64/aarch64.h (TARGET_SVE): + Add && !TARGET_GENERAL_REGS_ONLY. + (TARGET_SVE2): Add && TARGET_SVE. + (TARGET_SVE2_AES, TARGET_SVE2_BITPERM, TARGET_SVE2_SHA3, + TARGET_SVE2_SM4): Add && TARGET_SVE2. + * config/aarch64/aarch64-sve-builtins.h + (sve_switcher::m_old_general_regs_only): New member. + * config/aarch64/aarch64-sve-builtins.cc (check_required_registers): + New function. + (reported_missing_registers_p): New variable. + (check_required_extensions): Call check_required_registers before + return if all required extenstions are present. + (sve_switcher::sve_switcher): Save TARGET_GENERAL_REGS_ONLY in + m_old_general_regs_only and clear MASK_GENERAL_REGS_ONLY in + global_options.x_target_flags. + (sve_switcher::~sve_switcher): Set MASK_GENERAL_REGS_ONLY in + global_options.x_target_flags if m_old_general_regs_only is true. + 2020-04-22 Zackery Spytz * doc/extend.exi: Add "free" to list of other builtin functions diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc index ca4a0ebdd0c..8511382351c 100644 --- a/gcc/config/aarch64/aarch64-sve-builtins.cc +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc @@ -558,6 +558,10 @@ static hash_table *function_table; when the required extension is disabled. */ static bool reported_missing_extension_p; +/* True if we've already complained about attempts to use functions + which require registers that are missing. */ +static bool reported_missing_registers_p; + /* Record that TYPE is an ABI-defined SVE type that contains NUM_ZR SVE vectors and NUM_PR SVE predicates. MANGLED_NAME, if nonnull, is the ABI-defined mangling of the type. */ @@ -657,6 +661,29 @@ report_missing_extension (location_t location, tree fndecl, reported_missing_extension_p = true; } +/* Check whether the registers required by SVE function fndecl are available. + Report an error against LOCATION and return false if not. */ +static bool +check_required_registers (location_t location, tree fndecl) +{ + /* Avoid reporting a slew of messages for a single oversight. */ + if (reported_missing_registers_p) + return false; + + if (TARGET_GENERAL_REGS_ONLY) + { + /* SVE registers are not usable when -mgeneral-regs-only option + is specified. */ + error_at (location, + "ACLE function %qD is incompatible with the use of %qs", + fndecl, "-mgeneral-regs-only"); + reported_missing_registers_p = true; + return false; + } + + return true; +} + /* Check whether all the AARCH64_FL_* values in REQUIRED_EXTENSIONS are enabled, given that those extensions are required for function FNDECL. Report an error against LOCATION if not. */ @@ -666,7 +693,7 @@ check_required_extensions (location_t location, tree fndecl, { uint64_t missing_extensions = required_extensions & ~aarch64_isa_flags; if (missing_extensions == 0) - return true; + return check_required_registers (location, fndecl); static const struct { uint64_t flag; const char *name; } extensions[] = { #define AARCH64_OPT_EXTENSION(EXT_NAME, FLAG_CANONICAL, FLAGS_ON, FLAGS_OFF, \ @@ -851,6 +878,9 @@ sve_switcher::sve_switcher () aarch64_isa_flags = (AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_F16 | AARCH64_FL_SVE); + m_old_general_regs_only = TARGET_GENERAL_REGS_ONLY; + global_options.x_target_flags &= ~MASK_GENERAL_REGS_ONLY; + memcpy (m_old_have_regs_of_mode, have_regs_of_mode, sizeof (have_regs_of_mode)); for (int i = 0; i < NUM_MACHINE_MODES; ++i) @@ -862,6 +892,8 @@ sve_switcher::~sve_switcher () { memcpy (have_regs_of_mode, m_old_have_regs_of_mode, sizeof (have_regs_of_mode)); + if (m_old_general_regs_only) + global_options.x_target_flags |= MASK_GENERAL_REGS_ONLY; aarch64_isa_flags = m_old_isa_flags; } diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h b/gcc/config/aarch64/aarch64-sve-builtins.h index f7f06d26251..526d9f55e7b 100644 --- a/gcc/config/aarch64/aarch64-sve-builtins.h +++ b/gcc/config/aarch64/aarch64-sve-builtins.h @@ -658,6 +658,7 @@ public: private: unsigned long m_old_isa_flags; + bool m_old_general_regs_only; bool m_old_have_regs_of_mode[MAX_MACHINE_MODE]; }; diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 8f08bad3562..74236c3cffd 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -309,22 +309,22 @@ extern unsigned aarch64_architecture_version; #define TARGET_DOTPROD (TARGET_SIMD && AARCH64_ISA_DOTPROD) /* SVE instructions, enabled through +sve. */ -#define TARGET_SVE (AARCH64_ISA_SVE) +#define TARGET_SVE (!TARGET_GENERAL_REGS_ONLY && AARCH64_ISA_SVE) /* SVE2 instructions, enabled through +sve2. */ -#define TARGET_SVE2 (AARCH64_ISA_SVE2) +#define TARGET_SVE2 (TARGET_SVE && AARCH64_ISA_SVE2) /* SVE2 AES instructions, enabled through +sve2-aes. */ -#define TARGET_SVE2_AES (AARCH64_ISA_SVE2_AES) +#define TARGET_SVE2_AES (TARGET_SVE2 && AARCH64_ISA_SVE2_AES) /* SVE2 BITPERM instructions, enabled through +sve2-bitperm. */ -#define TARGET_SVE2_BITPERM (AARCH64_ISA_SVE2_BITPERM) +#define TARGET_SVE2_BITPERM (TARGET_SVE2 && AARCH64_ISA_SVE2_BITPERM) /* SVE2 SHA3 instructions, enabled through +sve2-sha3. */ -#define TARGET_SVE2_SHA3 (AARCH64_ISA_SVE2_SHA3) +#define TARGET_SVE2_SHA3 (TARGET_SVE2 && AARCH64_ISA_SVE2_SHA3) /* SVE2 SM4 instructions, enabled through +sve2-sm4. */ -#define TARGET_SVE2_SM4 (AARCH64_ISA_SVE2_SM4) +#define TARGET_SVE2_SM4 (TARGET_SVE2 && AARCH64_ISA_SVE2_SM4) /* ARMv8.3-A features. */ #define TARGET_ARMV8_3 (AARCH64_ISA_V8_3) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index b23a2dd423b..acd8b1ae133 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-04-22 Felix Yang + + PR target/94678 + * gcc.target/aarch64/sve/acle/general/nosve_6.c: New test. + 2020-04-22 José Rui Faustino de Sousa PR fortran/90350 diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c new file mode 100644 index 00000000000..d91ba40de14 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c @@ -0,0 +1,12 @@ +/* { dg-options "-march=armv8-a -mgeneral-regs-only" } */ + +#pragma GCC aarch64 "arm_sve.h" + +#pragma GCC target "+sve" + +void +f (svbool_t *x, svint8_t *y) +{ + *x = svptrue_b8 (); /* { dg-error {ACLE function '(svbool_t svptrue_b8\(\)|svptrue_b8)' is incompatible with the use of '-mgeneral-regs-only'} } */ + *y = svadd_m (*x, *y, 1); +} -- 2.30.2