Mostly LGTM, just a couple of minor points:
"Yangfei (Felix)" <felix.y...@huawei.com> writes:
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 6e226cc..e956b69 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,23 @@
> +2020-04-22 Felix Yang <felix.y...@huawei.com>
> +
> + PR target/94678
> + * config/aarch64/aarch64.h (TARGET_SVE, TARGET_SVE2):
> + Add && !TARGET_GENERAL_REGS_ONLY.
> + (TARGET_SVE2_AES, TARGET_SVE2_BITPERM, TARGET_SVE2_SHA3,
> + TARGET_SVE2_SM4): Add && TARGET_SVE2.
I agree using "TARGET_SVE2 &&" is just as good as adding
"!TARGET_GENERAL_REGS_ONLY &&", and arguably better. But I think
it would then be more consistent to add "TARGET_SVE &&" rather than
"!TARGET_GENERAL_REGS_ONLY &&" to TARGET_SVE2.
> @@ -558,6 +558,10 @@ static hash_table<registered_function_hasher>
> *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 is missing. */
s/is/are/
> +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,28 @@ report_missing_extension (location_t location, tree
> fndecl,
> reported_missing_extension_p = true;
> }
>
> +/* Check whether the registers required by SVE function fndecl are available.
> + SVE registers are not usable when -mgeneral-regs-only option is specified.
> + Report an error against LOCATION and return false if not. */
The "Report an error ..." sentence really forms a unit with the
"Check ..." sentence. I think it would probably be better to move
the intervening "SVE registers are not usable..." comment...
> +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)
...here.
> + {
> + 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. */
> [...]
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h
> b/gcc/config/aarch64/aarch64-sve-builtins.h
> index f7f06d2..5fdccb1 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 old_general_regs_only;
The convention (not always followed!) is to use an "m_" prefix for
private member variables for classes.
[https://gcc.gnu.org/codingconventions.html#Cxx_Names]
So please add an "m_" prefix here for consistency with the other two
member variables.
Thanks,
Richard
> 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 8f08bad..3aa3e25 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_GENERAL_REGS_ONLY && 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 be374fb..b8ed79c 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-04-22 Felix Yang <felix.y...@huawei.com>
> +
> + PR target/94678
> + * gcc.target/aarch64/sve/acle/general/nosve_6.c: New test.
> +
> 2020-04-22 Patrick Palka <ppa...@redhat.com>
>
> PR c++/67825
> 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 0000000..d91ba40
> --- /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);
> +}
