[PATCH] Fix aarch64 PR 99657: ICE with SVE types used without an error
Richard Sandiford
richard.sandiford@arm.com
Tue Nov 9 14:38:12 GMT 2021
apinski--- via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> From: Andrew Pinski <apinski@marvell.com>
>
> This fixes fully where SVE types were being used without sve being enabled.
> Instead of trying to fix it such that we error out during RTL time, it is
> better to error out in front-ends. This expands verify_type_context to
> have a context of auto storage decl which is used for both auto storage
> decls and for indirection context.
>
> A few testcases needed to be updated for the new error message; they were
> already being rejected before hand.
>
> OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions.
>
> PR target/99657
> gcc/c/ChangeLog:
>
> * c-decl.c (finish_decl): Call verify_type_context
> for all decls and not just global_decls.
> * c-typeck.c (build_indirect_ref): Call verify_type_context
> to check to see if the type is ok to be used.
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-sve-builtins.cc (verify_type_context):
> Add TXTC_AUTO_STORAGE support
> * target.h (enum type_context_kind): Add TXTC_AUTO_STORAGE.
>
> gcc/cp/ChangeLog:
>
> * decl.c (cp_finish_decl): Call verify_type_context
> for all decls and not just global_decls.
> * typeck.c (cp_build_indirect_ref_1): Call verify_type_context
> to check to see if the type is ok to be used.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/sve/acle/general/nosve_1.c: Update test.
> * gcc.target/aarch64/sve/acle/general/nosve_4.c: Likewise.
> * gcc.target/aarch64/sve/acle/general/nosve_5.c: Likewise.
> * gcc.target/aarch64/sve/acle/general/nosve_6.c: Likewise.
> * gcc.target/aarch64/sve/pcs/nosve_2.c: Likewise.
> * gcc.target/aarch64/sve/pcs/nosve_3.c: Likewise.
> * gcc.target/aarch64/sve/pcs/nosve_4.c: Likewise.
> * gcc.target/aarch64/sve/pcs/nosve_5.c: Likewise.
> * gcc.target/aarch64/sve/pcs/nosve_6.c: Likewise.
> * gcc.target/aarch64/sve/pcs/nosve_9.c: New test.
The AArch64 bits look good to me. I agree that this gives
better error messages than trying to track the error during
RTL time (although we still have to do that too in some cases).
Thanks,
Richard
> ---
> gcc/c/c-decl.c | 14 +++++++-------
> gcc/c/c-typeck.c | 2 ++
> gcc/config/aarch64/aarch64-sve-builtins.cc | 14 ++++++++++++++
> gcc/cp/decl.c | 10 ++++++----
> gcc/cp/typeck.c | 4 ++++
> gcc/target.h | 3 +++
> .../gcc.target/aarch64/sve/acle/general/nosve_1.c | 1 +
> .../gcc.target/aarch64/sve/acle/general/nosve_4.c | 2 +-
> .../gcc.target/aarch64/sve/acle/general/nosve_5.c | 2 +-
> .../gcc.target/aarch64/sve/acle/general/nosve_6.c | 1 +
> .../gcc.target/aarch64/sve/pcs/nosve_2.c | 2 +-
> .../gcc.target/aarch64/sve/pcs/nosve_3.c | 2 +-
> .../gcc.target/aarch64/sve/pcs/nosve_4.c | 3 +--
> .../gcc.target/aarch64/sve/pcs/nosve_5.c | 3 +--
> .../gcc.target/aarch64/sve/pcs/nosve_6.c | 3 +--
> .../gcc.target/aarch64/sve/pcs/nosve_9.c | 15 +++++++++++++++
> 16 files changed, 60 insertions(+), 21 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c
>
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 186fa1692c1..b3583622475 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -5441,19 +5441,19 @@ finish_decl (tree decl, location_t init_loc, tree init,
>
> if (VAR_P (decl))
> {
> + type_context_kind context = TCTX_AUTO_STORAGE;
> if (init && TREE_CODE (init) == CONSTRUCTOR)
> add_flexible_array_elts_to_size (decl, init);
>
> complete_flexible_array_elts (DECL_INITIAL (decl));
>
> if (is_global_var (decl))
> - {
> - type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
> - ? TCTX_THREAD_STORAGE
> - : TCTX_STATIC_STORAGE);
> - if (!verify_type_context (input_location, context, TREE_TYPE (decl)))
> - TREE_TYPE (decl) = error_mark_node;
> - }
> + context = (DECL_THREAD_LOCAL_P (decl)
> + ? TCTX_THREAD_STORAGE
> + : TCTX_STATIC_STORAGE);
> +
> + if (!verify_type_context (input_location, context, TREE_TYPE (decl)))
> + TREE_TYPE (decl) = error_mark_node;
>
> if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) != error_mark_node
> && COMPLETE_TYPE_P (TREE_TYPE (decl)))
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 782414f8c8c..e926b7c1964 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -2630,6 +2630,8 @@ build_indirect_ref (location_t loc, tree ptr, ref_operator errstring)
> else
> {
> tree t = TREE_TYPE (type);
> + if (!verify_type_context (loc, TCTX_AUTO_STORAGE, t))
> + return error_mark_node;
>
> ref = build1 (INDIRECT_REF, t, pointer);
>
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
> index bc92213665c..1d5083bf9fa 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> @@ -3834,11 +3834,25 @@ bool
> verify_type_context (location_t loc, type_context_kind context,
> const_tree type, bool silent_p)
> {
> + static bool informed = false;
> if (!sizeless_type_p (type))
> return true;
>
> switch (context)
> {
> + case TCTX_AUTO_STORAGE:
> + if (TARGET_SVE)
> + return true;
> + if (!silent_p && !informed)
> + {
> + informed = true;
> + error_at (loc, "SVE type %qT used without SVE ISA enabled", type);
> + inform (loc, "you can enable SVE using the command-line"
> + " option %<-march%>, or by using the %<target%>"
> + " attribute or pragma");
> + }
> + return false;
> +
> case TCTX_SIZEOF:
> case TCTX_STATIC_STORAGE:
> if (!silent_p)
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 947bbfc6637..1ffd11acae0 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -7975,11 +7975,13 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
> && DECL_INITIALIZED_IN_CLASS_P (decl))
> check_static_variable_definition (decl, type);
>
> - if (!processing_template_decl && VAR_P (decl) && is_global_var (decl))
> + if (!processing_template_decl && VAR_P (decl))
> {
> - type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
> - ? TCTX_THREAD_STORAGE
> - : TCTX_STATIC_STORAGE);
> + type_context_kind context = TCTX_AUTO_STORAGE;
> + if (is_global_var (decl))
> + context = (DECL_THREAD_LOCAL_P (decl)
> + ? TCTX_THREAD_STORAGE
> + : TCTX_STATIC_STORAGE);
> verify_type_context (input_location, context, TREE_TYPE (decl));
> }
>
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index cb20329ceb5..61122e160a9 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -3608,6 +3608,10 @@ cp_build_indirect_ref_1 (location_t loc, tree ptr, ref_operator errorstring,
> return TREE_OPERAND (pointer, 0);
> else
> {
> + if (!verify_type_context (loc, TCTX_AUTO_STORAGE, t,
> + !(complain & tf_error)))
> + return error_mark_node;
> +
> tree ref = build1 (INDIRECT_REF, t, pointer);
>
> /* We *must* set TREE_READONLY when dereferencing a pointer to const,
> diff --git a/gcc/target.h b/gcc/target.h
> index d8f45fb99c3..2570b775c5e 100644
> --- a/gcc/target.h
> +++ b/gcc/target.h
> @@ -217,6 +217,9 @@ const unsigned int VECT_COMPARE_COSTS = 1U << 0;
> /* The contexts in which the use of a type T can be checked by
> TARGET_VERIFY_TYPE_CONTEXT. */
> enum type_context_kind {
> + /* Creeating objects of type T with auto storage duration. */
> + TCTX_AUTO_STORAGE,
> +
> /* Directly measuring the size of T. */
> TCTX_SIZEOF,
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c
> index 09dfacd222d..3ea786fe4b3 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c
> @@ -7,6 +7,7 @@ f (svbool_t *x, svint8_t *y)
> {
> *x = svptrue_b8 (); /* { dg-error {ACLE function '(svbool_t svptrue_b8\(\)|svptrue_b8)' requires ISA extension 'sve'} } */
> /* { dg-message {note: you can enable 'sve' using the command-line option '-march', or by using the 'target' attribute or pragma} "" { target *-*-* } .-1 } */
> + /* { dg-error "SVE type 'svbool_t' {aka '__SVBool_t'} used without SVE ISA enabled" "indirect" {target *-*-* } .-2 } */
> *x = svptrue_b8 ();
> *x = svptrue_b8 ();
> *x = svptrue_b8 ();
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c
> index 35ab07f1b49..6aef1a77f14 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c
> @@ -3,6 +3,6 @@
> void
> f (__SVBool_t *x, __SVBool_t *y)
> {
> - *x = *y; /* { dg-error {this operation requires the SVE ISA extension} } */
> + *x = *y; /* { dg-error {SVE type '__SVBool_t' used without SVE ISA enabled} } */
> *x = *y;
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c
> index 6e8d951b294..33e2069fcb0 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c
> @@ -3,6 +3,6 @@
> void
> f (__SVInt8_t *x, __SVInt8_t *y)
> {
> - *x = *y; /* { dg-error {this operation requires the SVE ISA extension} } */
> + *x = *y; /* { dg-error {SVE type '__SVInt8_t' used without SVE ISA enabled} } */
> *x = *y;
> }
> 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
> index d91ba40de14..7dc10528a17 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c
> @@ -8,5 +8,6 @@ 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'} } */
> + /* { dg-error {SVE type 'svbool_t' {aka '__SVBool_t'} used without SVE ISA enabled} "" { target *-*-* } .-1 } */
> *y = svadd_m (*x, *y, 1);
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c
> index 663165f892d..0f1bad1734e 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c
> @@ -10,5 +10,5 @@ svbool_t return_bool ();
> void
> f (svbool_t *ptr)
> {
> - *ptr = return_bool (); /* { dg-error {'return_bool' requires the SVE ISA extension} } */
> + *ptr = return_bool (); /* { dg-error {SVE type 'svbool_t' used without SVE ISA enabled} } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c
> index 6d5823cfde1..2f9ebd827a7 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c
> @@ -10,5 +10,5 @@ svbool_t (*return_bool) ();
> void
> f (svbool_t *ptr)
> {
> - *ptr = return_bool (); /* { dg-error {calls to functions of type 'svbool_t\(\)' require the SVE ISA extension} } */
> + *ptr = return_bool (); /* { dg-error {SVE type 'svbool_t' used without SVE ISA enabled} } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c
> index a248bdbdbd9..979f98c11c8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c
> @@ -10,6 +10,5 @@ void take_svuint8 (svuint8_t);
> void
> f (svuint8_t *ptr)
> {
> - take_svuint8 (*ptr); /* { dg-error {this operation requires the SVE ISA extension} } */
> - /* { dg-error {'take_svuint8' requires the SVE ISA extension} "" { target *-*-* } .-1 } */
> + take_svuint8 (*ptr); /* { dg-error {SVE type 'svuint8_t' used without SVE ISA enabled} } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c
> index 6263b5acdec..990e28aae70 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c
> @@ -11,6 +11,5 @@ void take_svuint8_eventually (float, float, float, float,
> void
> f (svuint8_t *ptr)
> {
> - take_svuint8_eventually (0, 0, 0, 0, 0, 0, 0, 0, *ptr); /* { dg-error {this operation requires the SVE ISA extension} } */
> - /* { dg-error {arguments of type '(svuint8_t|__SVUint8_t)' require the SVE ISA extension} "" { target *-*-* } .-1 } */
> + take_svuint8_eventually (0, 0, 0, 0, 0, 0, 0, 0, *ptr); /* { dg-error {SVE type 'svuint8_t' used without SVE ISA enabled} } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c
> index 85b68bb3881..f11de7017a2 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c
> @@ -10,6 +10,5 @@ void unprototyped ();
> void
> f (svuint8_t *ptr)
> {
> - unprototyped (*ptr); /* { dg-error {this operation requires the SVE ISA extension} } */
> - /* { dg-error {arguments of type '(svuint8_t|__SVUint8_t)' require the SVE ISA extension} "" { target *-*-* } .-1 } */
> + unprototyped (*ptr); /* { dg-error {SVE type 'svuint8_t' used without SVE ISA enabled} } */
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c
> new file mode 100644
> index 00000000000..7b7f322fe8c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-prune-output "compilation terminated" } */
> +
> +#include <arm_sve.h>
> +
> +#pragma GCC target "+nosve"
> +
> +int
> +f ()
> +{
> + char a[12];
> + __SVInt8_t freq; /* { dg-error {SVE type '__SVInt8_t' used without SVE ISA enabled} } */
> + return __builtin_bcmp (&freq, &freq, 10);
> +}
> +
More information about the Gcc-patches
mailing list